From 0739996e87ed39c5c950a97c6f225652279c5968 Mon Sep 17 00:00:00 2001 From: Jonathan Wang Date: Mon, 22 May 2023 22:32:35 -0700 Subject: [PATCH] Squashed commit of the following: commit 6256db116057856cabd0ad179d114e18cc3707c7 Author: Jonathan Wang Date: Mon May 22 22:31:13 2023 -0700 chore: sync with release-0.3.0 and update CI commit 096e3703f856143faf66ca756b607f321d40e25f Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 21:48:28 2023 -0500 feat: add docs and assert with non-empty array checks (#53) commit fb7352aa8ce56e6762c38761697909fe2d7f2f30 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 21:39:19 2023 -0500 fix: ignore code block for doctest (#52) commit dcfee63df22e96e4f4f29cf3805606c3cb66f418 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 20:37:20 2023 -0500 feat: add Github CI running tests (#51) commit ae152e0cb1f6dd09e1042d63c999045316bf9bd8 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 20:20:31 2023 -0500 chore: add assert for query_cell_at_pos (#50) commit 7f429a464f6ced576dc8489b29d5438a1f1e60a3 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 18:51:56 2023 -0500 feat: `fixed_base::msm_par` handles identity point (#48) We still require fixed base points to be non-identity, but now handle the case when scalars may be zero or the final MSM value is identity point. commit 2d2c7ff5749cd9955375dfb159c16d9501ddd11a Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 17:41:17 2023 -0500 fix: minor code quality fixes (#47) commit 0fff063c2e49f9991b54b20b0a323d2a555af8a2 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 17:16:07 2023 -0500 feat: add `parallelize_in` helper function (#46) Multi-threading of witness generation is tricky because one has to ensure the circuit column assignment order stays deterministic. To ensure good developer experience / avoiding pitfalls, we provide a new helper function for this. Co-authored-by: Jonathan Wang commit 805a21cfa374db2de8d24ff30270b33f81397469 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 13:45:15 2023 -0500 feat: use strict ec ops more often (#45) * `msm` implementations now always use `ec_{add,sub}_unequal` in strict mode for safety * Add docs to `scalar_multiply` and a flag to specify when it's safe to turn off some strict assumptions commit 2c276b44d2f9d9a1f4a6adb9b079b176ef587455 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 13:25:50 2023 -0500 Better handling of EC point at infinity (#44) * feat: allow `msm_par` to return identity point * feat: handle point at infinity `multi_scalar_multiply` and `multi_exp_par` now handle point at infinity completely Add docs for `ec_add_unequal, ec_sub_unequal, ec_double_and_add_unequal` to specify point at infinity leads to undefined behavior commit 8e9032c2ff8fd6ee11d131893ed1b5d09937a187 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Mon May 22 03:52:52 2023 -0500 Use new types to validate input assumptions (#43) * feat: add new types `ProperUint` and `ProperCrtUint` To guard around assumptions about big integer representations * fix: remove unused `FixedAssignedCRTInteger` * feat: use new types for bigint and field chips New types now guard for different assumptions on non-native bigint arithmetic. Distinguish between: - Overflow CRT integers - Proper BigUint with native part derived from limbs - Field elements where inequality < modulus is checked Also add type to help guard for inequality check in ec_add_unequal_strict Rust traits did not play so nicely with references, so I had to switch many functions to move inputs instead of borrow by reference. However to avoid writing `clone` everywhere, we allow conversion `From` reference to the new type via cloning. * feat: use `ProperUint` for `big_less_than` * feat(ecc): add fns for assign private witness points that constrain point to lie on curve * fix: unnecessary lifetimes * chore: remove clones commit 01a8ac957d61eed9151c9018ce2cdf24b1dee93d Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Fri May 19 13:33:14 2023 -0500 fix: `FieldChip::divide` renamed `divide_unsafe` (#41) Add `divide` that checks denomintor is nonzero. Add documentation in cases where `divide_unsafe` is used. commit 4fdafabef5b4ec986fe8a22181c3b41d94a0d73f Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Fri May 19 01:23:53 2023 -0500 Add documentation for all debug_asserts (#40) feat: add documentation for all debug_asserts commit 98f8b1d175d780c622c3e28b1a78c6ed96fd3f1e Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Fri May 19 00:19:30 2023 -0500 fix: get_last_bit two errors (#39) 2 embarassing errors: * Witness gen for last bit was wrong (used xor instead of &) * `ctx.get` was called after `range_check` so it was getting the wrong cell commit 9298a781b91a1c2addbbc3cc545a4ab74995cd7f Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Thu May 18 22:09:54 2023 -0500 Guard `ScalarField` byte representations to always be little-endian (#38) fix: guard `ScalarField` to be little-endian commit b33af5b7973d17e4c5478804ba0b6eba418c5701 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Thu May 18 19:52:51 2023 -0500 fix: `log2_ceil(0)` should return `0` (#37) commit c685862918e16a9c885fc554b18c7b1ef5816bf7 Author: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Date: Thu May 18 19:46:07 2023 -0500 fix(ecdsa): allow u1*G == u2*PK case (#36) NOTE: current ecdsa requires `r, s` to be given as proper CRT integers TODO: newtypes to guard this assumption commit 16a3e9d26f30df3ff1faa999aca245b50332ea53 Author: PatStiles <33334338+PatStiles@users.noreply.github.com> Date: Thu May 18 16:35:11 2023 -0500 Feat: extend halo2 base test coverage (#35) * feat: add flex_gate_test.rs and pos add() test * feat: add pos sub() test * feat: add pos neg() test * feat: add pos mul() test * feat: add pos mul_add() test * feat: add pos mul_not() test * feat: add pos assert_bit * feat: add pos div_unsafe() test * feat: add pos assert_is_const test * feat: add pos inner_product() test * feat: add pos inner_product_left_last() test * feat: add pos inner_product_with_sums test * feat: add pos sum_products_with_coeff_and_var test * feat: add pos and() test * feat: add pos not() test * feat: add pos select() test * feat: add pos or_and() test * feat: add pos bits_to_indicator() test * feat: add pos idx_to_indicator() test * feat: add pos select_by_indicator() test * feat: add pos select_from_idx() test * feat: add pos is_zero() test * feat: add pos is_equal() test * feat: add pos num_to_bits() test * feat: add pos lagrange_eval() test * feat: add pos get_field_element() test * feat: add pos range_check() tests * feat: add pos check_less_than() test * feat: add pos check_less_than_safe() test * feat: add pos check_big_less_than_safe() test * feat: add pos is_less_than() test * feat: add pos is_less_than_safe() test * feat: add pos is_big_less_than_safe() test * feat: add pos div_mod() test * feat: add pos get_last_bit() test * feat: add pos div_mod_var() test * fix: pass slices into test functions not arrays * feat: Add pos property tests for flex_gate * feat: Add positive property tests for flex_gate * feat: add pos property tests for range_check.rs * feat: add neg pranking test for idx_to_indicator * fix: change div_mod_var test values * feat(refactor): refactor property tests * fix: fix neg test, assert_const, assert_bit * fix: failing prop tests * feat: expand negative testing is_less_than_failing * fix: Circuit overflow errors on neg tests * fix: prop_test_mul_not * fix: everything but get_last_bit & lagrange * fix: clippy * fix: set LOOKUP_BITS in range tests, make range check neg test more robust * fix: neg_prop_tests cannot prank inputs Inputs have many copy constraints; pranking initial input will cause all copy constraints to fail * fix: test_is_big_less_than_safe, 240 bits max * Didn't want to change current `is_less_than` implementation, which in order to optimize lookups for smaller bits, only works when inputs have at most `(F::CAPACITY // lookup_bits - 1) * lookup_bits` bits * fix: inline doc for lagrange_and_eval * Remove proptest for lagrange_and_eval and leave as todo * tests: add readme about serial execution --------- Co-authored-by: Jonathan Wang commit 44bc7443db48336de7f06a3784004000801ff08c Author: yuliakot <93175658+yuliakot@users.noreply.github.com> Date: Sat May 20 00:24:18 2023 -0500 Update ecdsa.rs commit 33fcccc832b25ded6ddca9712c681a1fcb25d065 Author: yuliakot <93175658+yuliakot@users.noreply.github.com> Date: Sat May 20 00:23:18 2023 -0500 Update ecdsa.rs commit c42a2efafc9c72d3f8183c18ca9131ef0988816a Author: yuliakot <93175658+yuliakot@users.noreply.github.com> Date: Sat May 20 00:22:51 2023 -0500 Update ecdsa.rs commit e9386b9fbaba2c9c7aeea27c6ce914c233027cc5 Author: yuliakot <93175658+yuliakot@users.noreply.github.com> Date: Sat May 20 00:20:23 2023 -0500 Update tests.rs commit 150b88f6e69051477ff4e8422fe1e308e8d13676 Author: yuliakot <93175658+yuliakot@users.noreply.github.com> Date: Sat May 20 00:18:37 2023 -0500 Update mod.rs commit da7d5b1671300d4988a1dc30e6e58869ec337fb5 Author: yulliakot Date: Fri May 19 23:57:48 2023 -0500 More ecdsa tests --- .github/workflows/ci.yml | 68 +++---- halo2-ecc/Cargo.toml | 1 + halo2-ecc/src/secp256k1/tests/ecdsa_tests.rs | 203 +++++++++++++++++++ halo2-ecc/src/secp256k1/tests/mod.rs | 1 + 4 files changed, 239 insertions(+), 34 deletions(-) create mode 100644 halo2-ecc/src/secp256k1/tests/ecdsa_tests.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9f9e0bbe..228ecb50 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,47 +1,47 @@ -name: Rust +name: Tests on: push: - branches: [ "main", "release-0.3.0" ] + branches: ["main", "release-0.3.0"] pull_request: - branches: [ "main" ] + branches: ["main"] env: CARGO_TERM_COLOR: always jobs: build: - runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - name: Build - run: cargo build --verbose - - name: Run halo2-base tests - run: | - cd halo2-base - cargo test -- --test-threads=1 - cd .. - - name: Run halo2-ecc tests MockProver - run: | - cd halo2-ecc - cargo test -- test_fp - cargo test -- test_fp12 - cargo test -- test_ecc - cargo test -- test_secp256k1_ecdsa - cargo test -- test_ec_add - cargo test -- test_fixed_base_msm - cargo test -- test_msm - cargo test -- test_pairing - cd .. - - name: Run halo2-ecc tests real prover - run: | - cd halo2-ecc - cargo test --release -- test_fp_assert_eq - cargo test --release -- --nocapture bench_secp256k1_ecdsa - cargo test --release -- --nocapture bench_ec_add - cargo test --release -- --nocapture bench_fixed_base_msm - cargo test --release -- --nocapture bench_msm - cargo test --release -- --nocapture bench_pairing - cd .. + - uses: actions/checkout@v3 + - name: Build + run: cargo build --verbose + - name: Run halo2-base tests + run: | + cd halo2-base + cargo test -- --test-threads=1 + cd .. + - name: Run halo2-ecc tests MockProver + run: | + cd halo2-ecc + cargo test -- test_fp + cargo test -- test_fp12 + cargo test -- test_ecc + cargo test -- test_secp256k1_ecdsa + cargo test -- test_ecdsa + cargo test -- test_ec_add + cargo test -- test_fixed_base_msm + cargo test -- test_msm + cargo test -- test_pairing + cd .. + - name: Run halo2-ecc tests real prover + run: | + cd halo2-ecc + cargo test --release -- test_fp_assert_eq + cargo test --release -- --nocapture bench_secp256k1_ecdsa + cargo test --release -- --nocapture bench_ec_add + cargo test --release -- --nocapture bench_fixed_base_msm + cargo test --release -- --nocapture bench_msm + cargo test --release -- --nocapture bench_pairing + cd .. diff --git a/halo2-ecc/Cargo.toml b/halo2-ecc/Cargo.toml index d5c9d056..2b03e1cb 100644 --- a/halo2-ecc/Cargo.toml +++ b/halo2-ecc/Cargo.toml @@ -14,6 +14,7 @@ rand_chacha = "0.3.1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" rayon = "1.6.1" +test-case = "3.1.0" # arithmetic ff = "0.12" diff --git a/halo2-ecc/src/secp256k1/tests/ecdsa_tests.rs b/halo2-ecc/src/secp256k1/tests/ecdsa_tests.rs new file mode 100644 index 00000000..27d4c1c6 --- /dev/null +++ b/halo2-ecc/src/secp256k1/tests/ecdsa_tests.rs @@ -0,0 +1,203 @@ +#![allow(non_snake_case)] +use crate::fields::FpStrategy; +use crate::halo2_proofs::{ + arithmetic::CurveAffine, + dev::MockProver, + halo2curves::bn256::Fr, + halo2curves::secp256k1::{Fp, Fq, Secp256k1Affine}, +}; +use crate::secp256k1::{FpChip, FqChip}; +use crate::{ + ecc::{ecdsa::ecdsa_verify_no_pubkey_check, EccChip}, + fields::{FieldChip, PrimeField}, +}; +use ark_std::{end_timer, start_timer}; +use halo2_base::gates::builder::{ + CircuitBuilderStage, GateThreadBuilder, MultiPhaseThreadBreakPoints, RangeCircuitBuilder, +}; + +use halo2_base::gates::RangeChip; +use halo2_base::utils::{biguint_to_fe, fe_to_biguint, modulus}; +use halo2_base::Context; +use rand::random; +use rand_core::OsRng; +use serde::{Deserialize, Serialize}; +use std::fs::File; +use test_case::test_case; + +#[derive(Clone, Copy, Debug, Serialize, Deserialize)] +struct CircuitParams { + strategy: FpStrategy, + degree: u32, + num_advice: usize, + num_lookup_advice: usize, + num_fixed: usize, + lookup_bits: usize, + limb_bits: usize, + num_limbs: usize, +} + +fn ecdsa_test( + ctx: &mut Context, + params: CircuitParams, + r: Fq, + s: Fq, + msghash: Fq, + pk: Secp256k1Affine, +) { + std::env::set_var("LOOKUP_BITS", params.lookup_bits.to_string()); + let range = RangeChip::::default(params.lookup_bits); + let fp_chip = FpChip::::new(&range, params.limb_bits, params.num_limbs); + let fq_chip = FqChip::::new(&range, params.limb_bits, params.num_limbs); + + let [m, r, s] = [msghash, r, s].map(|x| fq_chip.load_private(ctx, x)); + + let ecc_chip = EccChip::>::new(&fp_chip); + let pk = ecc_chip.assign_point(ctx, pk); + // test ECDSA + let res = ecdsa_verify_no_pubkey_check::( + &ecc_chip, ctx, pk, r, s, m, 4, 4, + ); + assert_eq!(res.value(), &F::one()); +} + +fn random_parameters_ecdsa() -> (Fq, Fq, Fq, Secp256k1Affine) { + let sk = ::ScalarExt::random(OsRng); + let pubkey = Secp256k1Affine::from(Secp256k1Affine::generator() * sk); + let msg_hash = ::ScalarExt::random(OsRng); + + let k = ::ScalarExt::random(OsRng); + let k_inv = k.invert().unwrap(); + + let r_point = Secp256k1Affine::from(Secp256k1Affine::generator() * k).coordinates().unwrap(); + let x = r_point.x(); + let x_bigint = fe_to_biguint(x); + + let r = biguint_to_fe::(&(x_bigint % modulus::())); + let s = k_inv * (msg_hash + (r * sk)); + + (r, s, msg_hash, pubkey) +} + +fn custom_parameters_ecdsa(sk: u64, msg_hash: u64, k: u64) -> (Fq, Fq, Fq, Secp256k1Affine) { + let sk = ::ScalarExt::from(sk); + let pubkey = Secp256k1Affine::from(Secp256k1Affine::generator() * sk); + let msg_hash = ::ScalarExt::from(msg_hash); + + let k = ::ScalarExt::from(k); + let k_inv = k.invert().unwrap(); + + let r_point = Secp256k1Affine::from(Secp256k1Affine::generator() * k).coordinates().unwrap(); + let x = r_point.x(); + let x_bigint = fe_to_biguint(x); + + let r = biguint_to_fe::(&(x_bigint % modulus::())); + let s = k_inv * (msg_hash + (r * sk)); + + (r, s, msg_hash, pubkey) +} + +fn ecdsa_circuit( + r: Fq, + s: Fq, + msg_hash: Fq, + pubkey: Secp256k1Affine, + params: CircuitParams, + stage: CircuitBuilderStage, + break_points: Option, +) -> RangeCircuitBuilder { + let mut builder = match stage { + CircuitBuilderStage::Mock => GateThreadBuilder::mock(), + CircuitBuilderStage::Prover => GateThreadBuilder::prover(), + CircuitBuilderStage::Keygen => GateThreadBuilder::keygen(), + }; + let start0 = start_timer!(|| format!("Witness generation for circuit in {stage:?} stage")); + ecdsa_test(builder.main(0), params, r, s, msg_hash, pubkey); + + let circuit = match stage { + CircuitBuilderStage::Mock => { + builder.config(params.degree as usize, Some(20)); + RangeCircuitBuilder::mock(builder) + } + CircuitBuilderStage::Keygen => { + builder.config(params.degree as usize, Some(20)); + RangeCircuitBuilder::keygen(builder) + } + CircuitBuilderStage::Prover => RangeCircuitBuilder::prover(builder, break_points.unwrap()), + }; + end_timer!(start0); + circuit +} + +#[test] +#[should_panic(expected = "assertion failed: `(left == right)`")] +fn test_ecdsa_msg_hash_zero() { + let path = "configs/secp256k1/ecdsa_circuit.config"; + let params: CircuitParams = serde_json::from_reader( + File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")), + ) + .unwrap(); + + let (r, s, msg_hash, pubkey) = custom_parameters_ecdsa(random::(), 0, random::()); + + let circuit = ecdsa_circuit(r, s, msg_hash, pubkey, params, CircuitBuilderStage::Mock, None); + MockProver::run(params.degree, &circuit, vec![]).unwrap().assert_satisfied(); +} + +#[test] +#[should_panic(expected = "assertion failed: `(left == right)`")] +fn test_ecdsa_private_key_zero() { + let path = "configs/secp256k1/ecdsa_circuit.config"; + let params: CircuitParams = serde_json::from_reader( + File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")), + ) + .unwrap(); + + let (r, s, msg_hash, pubkey) = custom_parameters_ecdsa(0, random::(), random::()); + + let circuit = ecdsa_circuit(r, s, msg_hash, pubkey, params, CircuitBuilderStage::Mock, None); + MockProver::run(params.degree, &circuit, vec![]).unwrap().assert_satisfied(); +} + +#[test] +fn test_ecdsa_random_valid_inputs() { + let path = "configs/secp256k1/ecdsa_circuit.config"; + let params: CircuitParams = serde_json::from_reader( + File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")), + ) + .unwrap(); + + let (r, s, msg_hash, pubkey) = random_parameters_ecdsa(); + + let circuit = ecdsa_circuit(r, s, msg_hash, pubkey, params, CircuitBuilderStage::Mock, None); + MockProver::run(params.degree, &circuit, vec![]).unwrap().assert_satisfied(); +} + +#[test_case(1, 1, 1; "")] +fn test_ecdsa_custom_valid_inputs(sk: u64, msg_hash: u64, k: u64) { + let path = "configs/secp256k1/ecdsa_circuit.config"; + let params: CircuitParams = serde_json::from_reader( + File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")), + ) + .unwrap(); + + let (r, s, msg_hash, pubkey) = custom_parameters_ecdsa(sk, msg_hash, k); + + let circuit = ecdsa_circuit(r, s, msg_hash, pubkey, params, CircuitBuilderStage::Mock, None); + MockProver::run(params.degree, &circuit, vec![]).unwrap().assert_satisfied(); +} + +#[test_case(1, 1, 1; "")] +fn test_ecdsa_custom_valid_inputs_negative_s(sk: u64, msg_hash: u64, k: u64) { + let path = "configs/secp256k1/ecdsa_circuit.config"; + let params: CircuitParams = serde_json::from_reader( + File::open(path).unwrap_or_else(|e| panic!("{path} does not exist: {e:?}")), + ) + .unwrap(); + + let (r, s, msg_hash, pubkey) = custom_parameters_ecdsa(sk, msg_hash, k); + let s = -s; + + let circuit = ecdsa_circuit(r, s, msg_hash, pubkey, params, CircuitBuilderStage::Mock, None); + MockProver::run(params.degree, &circuit, vec![]).unwrap().assert_satisfied(); +} diff --git a/halo2-ecc/src/secp256k1/tests/mod.rs b/halo2-ecc/src/secp256k1/tests/mod.rs index ecc8b287..cdd58dd8 100644 --- a/halo2-ecc/src/secp256k1/tests/mod.rs +++ b/halo2-ecc/src/secp256k1/tests/mod.rs @@ -1 +1,2 @@ pub mod ecdsa; +pub mod ecdsa_tests;