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.

Sign up for GitHub

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

Fix schnorr test #158

Merged
merged 3 commits into from
Sep 18, 2023
Merged

Fix schnorr test #158

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e45d16a
fix schnorr test
justcode740 Sep 14, 2023
f0bdeb0
fix infinite loop due to rnd not change, fmt
justcode740 Sep 15, 2023
33a1cd7
fix rng clone err
justcode740 Sep 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions halo2-ecc/src/secp256k1/tests/schnorr_signature.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ use crate::halo2_proofs::{
halo2curves::bn256::Fr,
halo2curves::secp256k1::{Fp, Fq, Secp256k1Affine},
};
use halo2_base::{halo2_proofs::arithmetic::Field, utils::testing::base_test};
use crate::secp256k1::{FpChip, FqChip};
use crate::{
ecc::{schnorr_signature::schnorr_verify_no_pubkey_check, EccChip},
fields::FieldChip,
};
use halo2_base::utils::BigPrimeField;
use halo2_base::utils::fe_to_biguint;
use num_bigint::BigUint;
use halo2_base::gates::RangeChip;
use halo2_base::utils::fe_to_biguint;
use halo2_base::utils::BigPrimeField;
use halo2_base::Context;
use halo2_base::{halo2_proofs::arithmetic::Field, utils::testing::base_test};
use num_bigint::BigUint;
use num_integer::Integer;
use rand::rngs::StdRng;
use rand_core::SeedableRng;
Expand All @@ -37,7 +37,7 @@ pub fn schnorr_signature_test<F: BigPrimeField>(
ctx: &mut Context<F>,
range: &RangeChip<F>,
params: CircuitParams,
input: SchnorrInput
input: SchnorrInput,
) -> F {
let fp_chip = FpChip::<F>::new(&range, params.limb_bits, params.num_limbs);
let fq_chip = FqChip::<F>::new(&range, params.limb_bits, params.num_limbs);
Expand All @@ -57,8 +57,7 @@ pub fn schnorr_signature_test<F: BigPrimeField>(
pub fn random_schnorr_signature_input(rng: &mut StdRng) -> SchnorrInput {
let sk = <Secp256k1Affine as CurveAffine>::ScalarExt::random(rng.clone());
Copy link
Contributor

Choose a reason for hiding this comment

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

The point of the random test is to use different seeded Rngs. So you should use the given rng but mutate it.

Since rng: &mut StdRng is already a mutable reference, I think you should replace all rng.clone() with just rng.

let pk = Secp256k1Affine::from(Secp256k1Affine::generator() * sk);
let msg_hash =
<Secp256k1Affine as CurveAffine>::ScalarExt::random(rng.clone());
let msg_hash = <Secp256k1Affine as CurveAffine>::ScalarExt::random(rng.clone());
Copy link
Contributor

Choose a reason for hiding this comment

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

rng


let mut k = <Secp256k1Affine as CurveAffine>::ScalarExt::random(rng.clone());

Expand All @@ -68,7 +67,7 @@ pub fn random_schnorr_signature_input(rng: &mut StdRng) -> SchnorrInput {
let mut y: &Fp = r_point.y();
// make sure R.y is even
while fe_to_biguint(y).mod_floor(&BigUint::from(2u64)) != BigUint::from(0u64) {
k = <Secp256k1Affine as CurveAffine>::ScalarExt::random(rng.clone());
k = <Secp256k1Affine as CurveAffine>::ScalarExt::random(StdRng::from_seed([0u8; 32]));
Copy link
Contributor

Choose a reason for hiding this comment

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

rng (otherwise k is the same on every run of this function)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, exactly, sry i was seriously concerned/worried by some stuff and not in good state/clear mind when writing this, this is a stupid mistake..

r_point = Secp256k1Affine::from(Secp256k1Affine::generator() * k).coordinates().unwrap();
x = r_point.x();
y = r_point.y();
Expand All @@ -77,7 +76,7 @@ pub fn random_schnorr_signature_input(rng: &mut StdRng) -> SchnorrInput {
let r = *x;
let s = k + sk * msg_hash;

SchnorrInput {r, s, msg_hash, pk}
SchnorrInput { r, s, msg_hash, pk }
}

pub fn run_test(input: SchnorrInput) {
Expand All @@ -88,9 +87,9 @@ pub fn run_test(input: SchnorrInput) {
.unwrap();

let res = base_test()
.k(params.degree)
.lookup_bits(params.lookup_bits)
.run(|ctx, range| schnorr_signature_test(ctx, range, params, input));
.k(params.degree)
.lookup_bits(params.lookup_bits)
.run(|ctx, range| schnorr_signature_test(ctx, range, params, input));
assert_eq!(res, Fr::ONE);
}

Expand Down Expand Up @@ -144,4 +143,4 @@ fn bench_secp256k1_schnorr() -> Result<(), Box<dyn std::error::Error>> {
)?;
}
Ok(())
}
}
18 changes: 6 additions & 12 deletions halo2-ecc/src/secp256k1/tests/schnorr_signature_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,9 @@ use rand::{random, rngs::StdRng};
use rand_core::SeedableRng;
use test_case::test_case;

use super::schnorr_signature::{SchnorrInput, run_test, random_schnorr_signature_input};
use super::schnorr_signature::{random_schnorr_signature_input, run_test, SchnorrInput};

fn custom_parameters_schnorr_signature(
sk: u64,
msg_hash: u64,
k: u64,
) -> SchnorrInput {
fn custom_parameters_schnorr_signature(sk: u64, msg_hash: u64, k: u64) -> SchnorrInput {
let sk = <Secp256k1Affine as CurveAffine>::ScalarExt::from(sk);
let pk = Secp256k1Affine::from(Secp256k1Affine::generator() * sk);
let msg_hash = <Secp256k1Affine as CurveAffine>::ScalarExt::from(msg_hash);
Expand All @@ -22,22 +18,20 @@ fn custom_parameters_schnorr_signature(
let r = *x;
let s = k + sk * msg_hash;

SchnorrInput {r, s, msg_hash, pk}
SchnorrInput { r, s, msg_hash, pk }
}

#[test]
#[should_panic(expected = "assertion failed: `(left == right)`")]
fn test_schnorr_signature_msg_hash_zero() {
let input =
custom_parameters_schnorr_signature(random::<u64>(), 0, random::<u64>());
let input = custom_parameters_schnorr_signature(random::<u64>(), 0, random::<u64>());
run_test(input);
}

#[test]
#[should_panic(expected = "assertion failed: `(left == right)`")]
fn test_schnorr_signature_private_key_zero() {
let input =
custom_parameters_schnorr_signature(0, random::<u64>(), random::<u64>());
let input = custom_parameters_schnorr_signature(0, random::<u64>(), random::<u64>());
run_test(input);
}

Expand All @@ -62,4 +56,4 @@ fn test_schnorr_signature_random_valid_inputs() {
fn test_schnorr_signature_custom_valid_inputs(sk: u64, msg_hash: u64, k: u64) {
let input = custom_parameters_schnorr_signature(sk, msg_hash, k);
run_test(input);
}
}