Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* feat: upgrade overall `halo2-base` API to support future multi-threaded assignments using our basic gate * WIP: currently `gates::flex_gate` is updated and passes basic test * BUG: `GateInstructions::idx_to_indicator` was missing a constraint to check that the indicator witness was equal to 1 when non-zero. * Previously the constraint ensured that `ind[i] = 0` when `idx != i` however `ind[idx]` could be anything!!! * update: working benches for `mul` and `inner_product` * feat: add `test_multithread_gates` * BUG: `get_last_bit` did not do an `assert_bit` check on the answer * this function was not used anywhere * fix: `builder::assign_*` was not handling cases where two gates overlap and there is a break point in that overlap * we need to copy a cell between columns to fix * feat: update `gates::range` to working tests and new API * In keygen mode, the `CircuitBuilder` will clone the `ThreadBuilder` instead of `take`ing it because the same circuit is used for both vk gen and pk gen. This could lead to more memory usage for pk gen. * fix: change `AssignedValue` type to `KeccakAssignedValue` for compatibility after halo2-base update * Initial version 0.3.0 of halo2-ecc (#12) * add multi-thread witness assignment support for `variable_base_msm` and `fixed_base_msm` * batch size 100 MSM witness generation went from 500ms -> 100ms * Sync with updates in `halo2_proofs_axiom` * `assign_advice` not longer returns `Result` so no more `unwrap` * Fix: assert uses of size hint in release mode (#13) * remove `size_hint` in `inner_product_simple` * change other uses of `size_hint` to follow with `assert_eq!` instead of `debug_assert_eq!` * Fix: bit decomposition edge cases (#14) * fix: change `debug_assert` in `decompose_u64_digits_limbs` to restrict `bit_len < 64` and `decompose_biguint` to `64 <= bit_len < 128` * add more comprehensive tests for above two functions * Initial checkpoint for halo2-ecc v0.3.0 (#15) * chore: clippy --fix * Feat/add readme (#4) * feat: add README * feat: re-enable `secp256k1` module with updated tests * chore: fix result println * chore: update Cargo halo2_proofs_axiom to axiom/dev branch * compatibility update with `halo2_proofs_axiom` Co-authored-by: Matthias Seitz <[email protected]> * Fix: make `GateThreadBuilder` compatible with external usage (#16) * chore: expose gate_builder.unknown * feat: `GateThreadBuilder::assign_all` takes assigned_{advices,constants} as input instead of new hashmap, in case we want to constrain equalities for cells not belonging to this builder * chore: update halo2-pse tag * fix: `GateThreadBuilder::assign_all` now returns `HashMap`s of assigned cells for external equality constraints (e.g., instance cells, `AssignedCells` from chips not using halo2-lib). * fix: `assign_all` was not assigning constants as desired: it was assigning a new constant per context. This leads to confusion and possible undesired consequences down the line. * Fix: under-constrained `idx_to_indicator` (#17) *fix(BUG): `GateChip::idx_to_indicator` still had soundness bug where at index `idx` the value could be 0 or 1 (instead of only 1) * feat: add some function documentation * test(idx_to_indicator): add comprehensive tests * both positive and negative tests * Fix: soundness error in `FpChip::assert_eq` due to typo (#18) * chore: update halo2-ecc version to 0.3.0 * fix(BUG): `FpChip::assert_equal` had `a` instead of `b` typo * feat: add tests for `FpChip::assert_eq` * positive and negative tests * Remove redundant code and prevent race conditions (#19) * feat: move `GateCircuitBuilder::synthesize` to `sub_synthesize` function which also returns the assigned advices. * reduces code duplication between `GateCircuitBuilder::synthesize` and `RangeCircuitBuilder::synthesize` and also makes it easier to assign public instances elsewhere (e.g., snark-verifier) * feat: remove `Mutex` to prevent non-deterministism * In variable and fixed base `msm_par` functions, remove use of `Mutex<GateThreadBuilder>` because even the `Mutex` is not thread- safe in the sense that: if you let `Mutex` decide order that `GateThreadBuilder` is unlocked, you may still add Contexts to the builder in a non-deterministic order. * fix: `fixed_base::msm_par` loading new zeros * In `msm_par` each parallelized context was loading a new zero via `ctx.load_zero()` * This led to using more cells than the non-parallelized version * In `fixed_base_msm_in`, the if statement depending on `rayon::current_number_threads` leads to inconsistent proving keys between different machines. This has been removed and now uses a fixed number `25`. * chore: use `info!` instead of `println` for params * Allow `assign_all` also if `witness_gen_only = true` * Fix: `inner_product_left_last` size hint (#25) * Add documentation for halo2-base (#27) * adds draft documentation for range.rs * draft docs for lib.rs, utiils.rs, builder.rs * fix: add suggested doc edits for range.rs * docs: add draft documentation for flex_gate.rs * fix: range.rs doc capitalization error * fix: suggested edits for utils.rs docs * fix: resolve comments for range.rs docs * fix: resolve comments on flex_gate.rs docs * fix: resolve comments for lib.rs, util.rs docs * fix: resolve comments for builder.rs docs * chore: use `info!` instead of `println` for params * Allow `assign_all` also if `witness_gen_only = true` * Fix: `inner_product_left_last` size hint (#25) * docs: minor fixes --------- Co-authored-by: PatStiles <[email protected]> * Smart Range Builder (#29) * feat: smart `RangeCircuitBuilder` Allow `RangeCircuitBuilder` to not create lookup table if it detects that there's nothing to look up. * feat: add `RangeWithInstanceCircuitBuilder` * Moved from `snark-verifier-sdk` * Also made this circuit builder smart so it doesn't load lookup table if not necessary * In particular this can also be used as a `GateWithInstanceCircuitBuilder` * chore: derive Eq for CircuitBuilderStage * fix: RangeConfig should not unwrap LOOKUP_BITS * fix: `div_mod_var` when `a_num_bits <= b_num_bits` (#31) * 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 <[email protected]> * 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 * fix: `log2_ceil(0)` should return `0` (#37) * Guard `ScalarField` byte representations to always be little-endian (#38) fix: guard `ScalarField` to be little-endian * 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 * Add documentation for all debug_asserts (#40) feat: add documentation for all debug_asserts * fix: `FieldChip::divide` renamed `divide_unsafe` (#41) Add `divide` that checks denomintor is nonzero. Add documentation in cases where `divide_unsafe` is used. * 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 * 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 * 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 * 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 <[email protected]> * fix: minor code quality fixes (#47) * 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. * chore: add assert for query_cell_at_pos (#50) * feat: add Github CI running tests (#51) * fix: ignore code block for doctest (#52) * feat: add docs and assert with non-empty array checks (#53) * Release 0.3.0 ecdsa tests (#54) * More ecdsa tests * Update mod.rs * Update tests.rs * Update ecdsa.rs * Update ecdsa.rs * Update ecdsa.rs * chore: sync with release-0.3.0 and update CI Co-authored-by: yulliakot <[email protected]> Co-authored-by: yuliakot <[email protected]> * chore: fix CI cannot multi-thread tests involving lookups due to environment variables * fix: `prop_test_is_less_than_safe` (#58) This test doesn't run any prover so the input must satisfy range check assumption. More serious coverage is provided by `prop_test_neg_is_less_than_safe` * Add halo2-base readme (#66) * feat: add halo2-base readme * fix: readme formatting * fix: readme edits * fix: grammer * fix: use relative links and formatting * fix: formatting * feat: add RangeCircuitBuilder description * feat: rewording and small edits --------- Co-authored-by: PatStiles <[email protected]> * fix: change all `1` to `1u64` to prevent unexpected overflow (#72) * [Fix] Panic when dealing with identity point (#71) * More ecdsa tests * Update mod.rs * Update tests.rs * Update ecdsa.rs * Update ecdsa.rs * Update ecdsa.rs * msm tests * Update mod.rs * Update msm_sum_infinity.rs * fix: ec_sub_strict was panicing when output is identity * affects the MSM functions: right now if the answer is identity, there will be a panic due to divide by 0 instead of just returning 0 * there could be a more optimal solution, but due to the traits for EccChip, we just generate a random point solely to avoid divide by 0 in the case of identity point * Fix/fb msm zero (#77) * fix: fixed_base scalar multiply for [-1]P * feat: use `multi_scalar_multiply` instead of `scalar_multiply` * to reduce code maintanence / redundancy * fix: add back scalar_multiply using any_point * feat: remove flag from variable base `scalar_multiply` * feat: add scalar multiply tests for secp256k1 * fix: variable scalar_multiply last select * Fix/msm tests output identity (#75) * fixed base msm tests for output infinity * fixed base msm tests for output infinity --------- Co-authored-by: yulliakot <[email protected]> * feat: add tests and update CI --------- Co-authored-by: yuliakot <[email protected]> Co-authored-by: yulliakot <[email protected]> --------- Co-authored-by: yulliakot <[email protected]> Co-authored-by: yuliakot <[email protected]> * [Fix] scalar multiply completeness (#82) * fix: replace `scalar_multiply` with passthrough to MSM for now * feat(msm): use strict mode always * Previously did not use strict because we make assumptions about the curve `C`. Since this was not documented and is easy to miss, we use strict mode always. * docs: add assumptions to ec_sub_strict (#84) * fix: readme from previous merge * chore: cleanup CI for merge into main * chore: fix readme --------- Co-authored-by: Jonathan Wang <[email protected]> Co-authored-by: Matthias Seitz <[email protected]> Co-authored-by: PatStiles <[email protected]> Co-authored-by: PatStiles <[email protected]> Co-authored-by: yulliakot <[email protected]> Co-authored-by: yuliakot <[email protected]>
- Loading branch information
7 people
authored
Jun 20, 2023
1 parent
9860acc
commit ca8e11c