Skip to content
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

Merged
merged 7 commits into from
Apr 20, 2024
Merged

Schnorr's DLog PoK #11

merged 7 commits into from
Apr 20, 2024

Conversation

supragya
Copy link
Owner

@supragya supragya commented Apr 9, 2024

Summary by CodeRabbit

  • New Features

    • Introduced Schnorr's Identification Protocol and added related cryptographic primitives to enhance security and privacy.
  • Documentation

    • Detailed explanation of Schnorr's zero-knowledge protocol in the README.
    • Added future implementation plans in ToDo.md.
  • Bug Fixes

    • Ensured secrecy in cryptographic operations with additive blinding factors.

Copy link
Contributor

coderabbitai bot commented Apr 9, 2024

Walkthrough

The 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

File Path Change Summary
Cargo.toml Added schnorr-discrete-log-proof-of-knowledge, ark-crypto-primitives to members.
schnorr-discrete-log.../README.md Introduced Schnorr's Identification Protocol, details on interactive protocol and secrecy.
schnorr-discrete-log.../src/lib.rs Added structures and methods for Schnorr signatures and proof of knowledge using BLS12-381 curve.
README.md Updated with reference to Schnorr's DLog PoK.
ToDo.md Outlined future implementations including various cryptographic trees and polynomial domain extension.

🐇🌟
In the realm of code, where secrets are kept,
A rabbit hopped in, its promises adept.
With Schnorr's wise protocol now in place,
Secrets stay hidden, with nary a trace.
Hop, hop, hooray, let the crypto enhance,
In the burrows of blockchain, we take our stance!
🌟🐇


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

ToDo.md Show resolved Hide resolved
ToDo.md Show resolved Hide resolved
[Sch91]schnorr-discrete-log-proof-of-knowledge/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

supragya and others added 3 commits April 20, 2024 23:41
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Comment on lines 1 to 2
# 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.
Copy link
Contributor

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.

Suggested change
# 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.

Comment on lines 9 to 11
## References
- https://crypto.stanford.edu/cs355/19sp/lec5.pdf
- https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve
Copy link
Contributor

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.

Suggested change
## 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();
Copy link
Contributor

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.

Suggested change
let secret_key = C::ScalarField::from_bigint(secret.into()).unwrap();
let secret_key = C::ScalarField::from_bigint(secret.into()).ok_or(Error::new())?;

Comment on lines +57 to +82
#[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);
}
Copy link
Contributor

@coderabbitai coderabbitai bot Apr 20, 2024

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?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

Copy link
Contributor

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:

  1. Test with Zero Secret: Ensures the system handles zero as a secret key correctly.
  2. Test with Negative Secret: Although not typical in unsigned types, simulating a negative input can help ensure robustness.
  3. Test with Maximum Secret: Tests the upper boundary of the secret key space.
  4. 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!

@supragya supragya merged commit d27695d into main Apr 20, 2024
8 checks passed
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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
Copy link
Contributor

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.

Suggested change
# Schnorr's Identification Protocol
# Schnorr's Identification Protocol

Comment on lines +9 to +10
## References
## References
Copy link
Contributor

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.

Suggested change
## 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
Copy link
Contributor

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.

Suggested change
- https://crypto.stackexchange.com/questions/58954/multiplication-of-two-points-belong-to-elliptic-curve

@supragya supragya deleted the supragya/shnorr-dlog-pok branch April 20, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant