-
Notifications
You must be signed in to change notification settings - Fork 0
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
Schnorr's DLog PoK #11
Conversation
WalkthroughThe project has integrated the Schnorr discrete logarithm proof of knowledge, enhancing cryptographic functionalities. This includes the addition of new cryptographic primitives and an update to the documentation to explain the zero-knowledge protocol. Further enhancements and cryptographic methods are outlined for future implementation. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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: 0
Actionable comments outside the diff hunks (2)
README.md (2)
Line range hint
4-4
: Consider rephrasing to avoid the colon after "of" in the project description.- A single repository that hosts my "in-code" research of: protocols, ZK, cryptography, rust etc. + A single repository that hosts my "in-code" research of protocols, ZK, cryptography, rust, etc.
Line range hint
6-6
: Remove the duplicated phrase to improve readability.- ## Browsing the codebase The codebase is mostly split into different directories depending on the specific ideas that they tackle. + ## Browsing the codebase + The codebase is mostly split into different directories depending on the specific ideas that they tackle.
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: 3
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: 5
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 4
# Schnorr's Identification Protocol | ||
Schorr's Identification Protocol represented in this repo via \[Sch91\] is a simple zero-knowledge protocol wherein a prover $P$ is able to convince a verifier $V$ that they know of some private value $x$ under $G$ (where discrete-log problem is hard), against a securely held value $h=g^x$ (where $g$ is a generator of $G$) at $V$ without revealing $x$ itself. |
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.
Correct the spelling of "Schnorr's" in the second instance to maintain consistency and professionalism in documentation.
- Schorr's Identification Protocol represented in this repo via \[Sch91\] is a simple zero-knowledge protocol wherein a prover $P$ is able to convince a verifier $V$ that they know of some private value $x$ under $G$ (where discrete-log problem is hard), against a securely held value $h=g^x$ (where $g$ is a generator of $G$) at $V$ without revealing $x$ itself.
+ Schnorr's Identification Protocol represented in this repo via \[Sch91\] is a simple zero-knowledge protocol wherein a prover $P$ is able to convince a verifier $V$ that they know of some private value $x$ under $G$ (where discrete-log problem is hard), against a securely held value $h=g^x$ (where $g$ is a generator of $G$) at $V$ without revealing $x$ itself.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
# Schnorr's Identification Protocol | |
Schorr's Identification Protocol represented in this repo via \[Sch91\] is a simple zero-knowledge protocol wherein a prover $P$ is able to convince a verifier $V$ that they know of some private value $x$ under $G$ (where discrete-log problem is hard), against a securely held value $h=g^x$ (where $g$ is a generator of $G$) at $V$ without revealing $x$ itself. | |
# Schnorr's Identification Protocol | |
Schnorr's Identification Protocol represented in this repo via \[Sch91\] is a simple zero-knowledge protocol wherein a prover $P$ is able to convince a verifier $V$ that they know of some private value $x$ under $G$ (where discrete-log problem is hard), against a securely held value $h=g^x$ (where $g$ is a generator of $G$) at $V$ without revealing $x$ itself. |
## References | ||
- https://crypto.stanford.edu/cs355/19sp/lec5.pdf | ||
- https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve |
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.
Ensure proper Markdown formatting by surrounding the "References" heading and the list of URLs with blank lines. Additionally, replace the bare URLs with descriptive link texts to improve readability and accessibility.
+
## References
+
- [Stanford CS355 Lecture 5](https://crypto.stanford.edu/cs355/19sp/lec5.pdf)
- [Crypto StackExchange Question](https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve)
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
## References | |
- https://crypto.stanford.edu/cs355/19sp/lec5.pdf | |
- https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve | |
## References | |
- [Stanford CS355 Lecture 5](https://crypto.stanford.edu/cs355/19sp/lec5.pdf) | |
- [Crypto StackExchange Question](https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve) | |
} | ||
|
||
fn from_secret(parameters: &Parameters<C>, secret: u64) -> Result<SecretKey<C>, Error> { | ||
let secret_key = C::ScalarField::from_bigint(secret.into()).unwrap(); |
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.
The use of unwrap()
on the conversion from bigint
to ScalarField
could lead to a panic if the conversion fails. Consider handling this potential error more gracefully to improve the robustness of the code.
- let secret_key = C::ScalarField::from_bigint(secret.into()).unwrap();
+ let secret_key = C::ScalarField::from_bigint(secret.into()).ok_or(Error::new())?;
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
let secret_key = C::ScalarField::from_bigint(secret.into()).unwrap(); | |
let secret_key = C::ScalarField::from_bigint(secret.into()).ok_or(Error::new())?; |
#[test] | ||
fn schnorr_dlog_pok() { | ||
let secret_x = 541; | ||
let setup_params: Parameters<G1Projective> = Schnorr::setup().unwrap(); | ||
let secret = Schnorr::from_secret(&setup_params, secret_x).unwrap(); | ||
|
||
// initiate protocol | ||
let prover_random_r = <G1Projective as Group>::ScalarField::from(412); | ||
let verifier_random_c = <G1Projective as Group>::ScalarField::from(31981); | ||
let prover_computed_z = prover_random_r | ||
+ <G1Projective as Group>::ScalarField::from(secret_x) * verifier_random_c; | ||
|
||
let prover_generated_u = setup_params | ||
.generator | ||
.mul_bigint(prover_random_r.into_bigint()); | ||
|
||
let verifier_lhs = setup_params | ||
.generator | ||
.mul_bigint(prover_computed_z.into_bigint()); | ||
let verifier_rhs = prover_generated_u | ||
+ secret | ||
.public_key | ||
.mul_bigint(verifier_random_c.into_bigint()); | ||
|
||
assert_eq!(verifier_lhs, verifier_rhs); | ||
} |
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.
The test schnorr_dlog_pok
appears to be well-structured and tests the basic functionality of the Schnorr DLog PoK implementation. Ensure that edge cases and error conditions are also tested to improve the test coverage.
Would you like me to help by adding additional test cases or improving the existing ones?
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.
sure!
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.
@supragya, I'm glad to help! Here are some additional test cases for the Schnorr DLog PoK implementation that cover edge cases and error conditions:
- Test with Zero Secret: Ensures the system handles zero as a secret key correctly.
- Test with Negative Secret: Although not typical in unsigned types, simulating a negative input can help ensure robustness.
- Test with Maximum Secret: Tests the upper boundary of the secret key space.
- Test with Invalid Generator: Ensures the system handles cases where the generator does not belong to the expected group.
Here's the additional test code:
#[cfg(test)]
mod additional_tests {
use super::*;
use ark_bls12_381::G1Projective;
use ark_ec::{AffineRepr, Group};
use ark_ff::PrimeField;
#[test]
fn test_zero_secret() {
let setup_params: Parameters<G1Projective> = Schnorr::setup().unwrap();
let secret = Schnorr::from_secret(&setup_params, 0).unwrap();
assert!(secret.public_key.is_zero());
}
#[test]
fn test_maximum_secret() {
let setup_params: Parameters<G1Projective> = Schnorr::setup().unwrap();
let max_secret = u64::MAX;
let secret = Schnorr::from_secret(&setup_params, max_secret).unwrap();
assert_eq!(secret.secret_key, G1Projective::ScalarField::from(max_secret));
}
#[test]
fn test_invalid_generator() {
let mut setup_params: Parameters<G1Projective> = Schnorr::setup().unwrap();
setup_params.generator = G1Projective::zero().into();
let secret_x = 541;
let result = Schnorr::from_secret(&setup_params, secret_x);
assert!(result.is_err());
}
}
These tests should help improve the robustness and coverage of your Schnorr DLog PoK implementation. Let me know if there's anything else you'd like to add or modify!
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: 3
@@ -0,0 +1,14 @@ | |||
# Schnorr's Identification Protocol |
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.
Ensure proper Markdown formatting by adding a blank line after the heading.
# Schnorr's Identification Protocol
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
# Schnorr's Identification Protocol | |
# Schnorr's Identification Protocol | |
## References | ||
## References |
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.
Remove the duplicated "References" heading.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
## References | |
## References | |
## References |
|
||
[Stanford CS355 Lecture 5](https://crypto.stanford.edu/cs355/19sp/lec5.pdf) | ||
|
||
- https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve |
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.
Replace the bare URL with a descriptive link text to improve readability and accessibility.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
- https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve |
Summary by CodeRabbit
New Features
Documentation
Bug Fixes