[zk-sdk] Add fixed string tests for proofs #2814
Merged
Conversation
This file contains 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
joncinque
reviewed
Sep 3, 2024
zk-sdk/src/range_proof/mod.rs
Outdated
Comment on lines
535
to
541
| assert!(proof | ||
| .verify( | ||
| vec![&commitment_1, &commitment_2, &commitment_3], | ||
| vec![64, 32, 32], | ||
| &mut transcript_verify, | ||
| ) | ||
| .is_ok()); |
There was a problem hiding this comment.
nit: up to you, but I typically lean towards result.unwrap() instead of assert!(result.is_ok()), so that if it fails, you get a better error message
Comment on lines
229
to
241
| assert!(proof | ||
| .verify( | ||
| &first_pubkey, | ||
| &second_pubkey, | ||
| &commitment_lo, | ||
| &commitment_hi, | ||
| &first_handle_lo, | ||
| &first_handle_hi, | ||
| &second_handle_lo, | ||
| &second_handle_hi, | ||
| &mut verifier_transcript, | ||
| ) | ||
| .is_ok()); |
There was a problem hiding this comment.
nit: same here, prefer unwrap() over assert!(...is_ok())
|
|
||
| let mut verifier_transcript = Transcript::new(b"Test"); | ||
|
|
||
| assert!(proof |
|
|
||
| let mut verifier_transcript = Transcript::new(b"Test"); | ||
|
|
||
| assert!(proof |
|
|
||
| let mut verifier_transcript = Transcript::new(b"Test"); | ||
|
|
||
| assert!(proof |
Comment on lines
315
to
329
| let pod_commitment: PodPedersenCommitment = commitment.into(); | ||
| println!("commitment: {}", pod_commitment); | ||
| println!("first_pubkey: {}", first_pubkey); | ||
| println!("second_pubkey: {}", second_pubkey); | ||
| println!("third_pubkey: {}", third_pubkey); | ||
|
|
||
| let pod_first_handle: PodDecryptHandle = first_handle.into(); | ||
| let pod_second_handle: PodDecryptHandle = second_handle.into(); | ||
| let pod_third_handle: PodDecryptHandle = third_handle.into(); | ||
|
|
||
| println!("first_handle: {}", pod_first_handle); | ||
| println!("second_handle: {}", pod_second_handle); | ||
| println!("third_handle: {}", pod_third_handle); | ||
| let pod_proof: PodGroupedCiphertext3HandlesValidityProof = proof.clone().into(); | ||
| println!("pod proof: {}", pod_proof); |
|
|
||
| let mut verifier_transcript = Transcript::new(b"Test"); | ||
|
|
||
| assert!(proof |
|
|
||
| let mut verifier_transcript = Transcript::new(b"test"); | ||
|
|
||
| assert!(proof |
|
|
||
| let mut verifier_transcript = Transcript::new(b"test"); | ||
|
|
||
| assert!(proof.verify(&pubkey, &mut verifier_transcript).is_ok()); |
|
|
||
| let mut verifier_transcript = Transcript::new(b"test"); | ||
|
|
||
| assert!(proof |
joncinque
approved these changes
Sep 4, 2024
ray-kast
pushed a commit
to abklabs/agave
that referenced
this pull request
Nov 27, 2024
* refactor `impl_from_str` and `impl_from_bytes` macros * impl `Display`, `FromStr`, `From<[u8; _]>` for sigma and range proof pod types * impl `Display` for Pedersen commitment and grouped ElGamal ciphertext pod types * impl `Display`, `FromStr`, `From<[u8; _]>` for decrypt handle pod type * add fixed proof string tests * cargo clippy * remove println's * update `assert!` and `.is_ok()` with `unwrap`'s
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.
Problem
Currently, all the tests for the sigma and range proofs in zk-sdk verify fresh proofs that are randomly generated using the proof constructors. Although these tests are useful for testing the correctness of the proof generation and verification logic, they fail to detect potential changes to the code that are not backwards compatible.
For example, suppose that there is a breaking change in the curve25519-dalek crate (e.g. a group generator constant was updated). An upgrade to the curve25519-dalek crate would apply to both the proof generation and verification code so that their correctness is maintained. However, the old proofs (new proofs) will not verify with respect to the new verification logic (old verification logic).
Summary of Changes
I added tests that deserializes a string representation of proofs and verifies them. In the process, I implemented
Display,FromStr, andFrom<[u8; _]>for the pod proof types as well as some of the ElGamal types, which would be useful more generally as well.Fixes #