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

Support Column annotations for better debugging experience #705

Open
CPerezz opened this issue Dec 8, 2022 · 7 comments · May be fixed by #706
Open

Support Column annotations for better debugging experience #705

CPerezz opened this issue Dec 8, 2022 · 7 comments · May be fixed by #706
Labels
A-dev-tooling Area: Developer tooling A-errors Area: Error handling

Comments

@CPerezz
Copy link
Contributor

CPerezz commented Dec 8, 2022

Hey. In PSE we are sometimes having bad times debugging zkevm-circuits. The thing resides on the fact that we get traces that do not contain the names of the Columns mainly.

I thought that would be nice to have a way on which we can annotate columns and later on, get the annotations as helpful debug info with the MockProver.

To put an easy example, I wish we could get something like:

  Cell layout in region 'Faulty synthesis':
    | Offset |This is annotated!|This is also annotated!|
    +--------+------------------+-----------------------+
    |    0   |        x0        |                       |
    |    1   |                  |           X           | <--{ X marks the spot! 🦜

  Gate 'Equality check' (applied at offset 1) queries these cells.

Instead of having Columns called A0 or I3 and so on.. which is the current behaviour.

@CPerezz
Copy link
Contributor Author

CPerezz commented Dec 8, 2022

Initial exploration/discussion at #706

@str4d
Copy link
Contributor

str4d commented Dec 9, 2022

Generally +1 on annotating columns somehow. More debugging annotations are always useful!

It's likely going to cause the CLI dev tooling output to get rather unwieldy though; this is where having a GUI or web-based debugging interface would help, as these could be rendered as annotations.

@str4d
Copy link
Contributor

str4d commented Dec 9, 2022

An alternative way to render the column annotations (either always, or as an option) would be to provide a map underneath from the A0-style identifiers to the annotations.

In any case, collecting the annotations (in a similar way to how we collect similar annotations everywhere else, via closures) is definitely something we should do, regardless of how they are rendered.

@str4d str4d added A-dev-tooling Area: Developer tooling A-errors Area: Error handling labels Dec 10, 2022
@CPerezz
Copy link
Contributor Author

CPerezz commented Dec 12, 2022

I've extended it a bit. Now the ConstraintNotSatisfied case is also correctly handled as seen here:

  Cell layout in region 'Wrong synthesis':
    | Offset |This is Advice!|This is Advice too!|Another one!|This is a Fixed!|
    +--------+---------------+-------------------+------------+----------------+
    |    0   |       x0      |         x1        |     x2     |       x3       | <--{ Gate 'Equality check' applied here

  Constraint '':
    (S1 * (x0 - x1)) * (x2 - x3) = 0

  Assigned cell values:
    x0 = 1
    x1 = 0
    x2 = 0x5
    x3 = 0x7

The main things left to do are:

  • Apply labeling for lookup columns and figure out how to handle global vs regional columns and them annotations
  • Fix the prover.verify() output to display the column annotations decently.

@CPerezz
Copy link
Contributor Author

CPerezz commented Dec 13, 2022

After 5e88f12 the render of Lookup table columns is enabled. Kudos to @ed255 for the help!!!
This is the current output for the lookups where we show the column that we fail to make the lookup to + the columns involved in the expression that we're looking up at.

error: lookup input does not exist in table
  (L0) ∉ (AnnotatedTable1)

  Lookup inputs:
    L0 = x1 * x0 + (1 - x1) * 0x2
    ^

    | Cell layout in region 'Faulty synthesis':
    |   | Offset |Witness example| F1 |
    |   +----------+-------------------------+-----+
    |   |      1    |               x0            | x1 | <--{ Lookup inputs queried here
    |
    | Assigned cell values:
    |   x0 = 0x5
    |   x1 = 1

The last challenge now is to fix the regular prover.verify() outputs. Since, as you can see here, this is the error we currently have:

thread 'dev::tests::bad_lookup' panicked at 'assertion failed: `(left == right)`
  left: `Err([Lookup { lookup_index: 0, location: InRegion { region: Region 2 ('Faulty synthesis'), offset: 1 } }])`,
 right: `Err([Lookup { lookup_index: 0, location: InRegion { region: Region 2 ('Faulty synthesis'), offset: 1 } }])`', halo2_proofs/src/dev.rs:1150:9

The issue here is that the VerifyFailure contains a LocationFailure which at the same time contains a metadata::Region. And this IS the issue. The HashMap with the Region allocations is not displayed. But the Eq and PartialEq impls which are derived via macros indeed consider the HashMap. Which is None for the hand-made one, and Some(HASHMAP_WITH_THE_ANNOTATIONS) for the fn-made one.

The idea here is:

  1. Manually implement Eq and PartialEq for metadata::Region<'a>. So that the HashMap of the annotations is not considered to constraint equality.
  2. (And this might be a bit larger), a re-implementation of Display for VerifyFailure struct will be needed so that we can see the error like:
 left: `Err([Lookup { lookup_index: 0, annotation: "Annotation", location: InRegion { region: Region 2 ('Faulty synthesis'), offset: 1 } }])`,

Or the one that would be really helpful:

ConstraintNotSatisfied {
            constraint: Constraint {
                gate: Gate {
                    index: 0,
                    name: "Equality check",
                },
                index: 0,
                name: "",
            },
            location: InRegion {
                region: Region 1 ('Wrong synthesis'),
                offset: 0,
            },
            cell_values: [
                (
                    VirtualCell {
                        name: "",
                        column: Column {
                            column_type: Advice,
                            index: 0,
                            annotation: "This is annotated!"
                        },
                        rotation: 0,
                    },
                    "1",
                ),
                (
                    VirtualCell {
                        name: "",
                        column: Column {
                            column_type: Advice,
                            index: 1,
                            annotation: "This is annotated too"
                        },
                        rotation: 0,
                    },
                    "0",
                ),
                (
                    VirtualCell {
                        name: "",
                        column: Column {
                            column_type: Advice,
                            index: 2,
                            annotation: ""
                        },
                        rotation: 0,
                    },
                    "0x5",
                ),
                (
                    VirtualCell {
                        name: "",
                        column: Column {
                            column_type: Fixed,
                            index: 0,
                            annotation: "FixedColAnn"
                        },
                        rotation: 0,
                    },
                    "0x7",
                ),
            ],
        },

Would you be Ok with these two points @str4d ?? I think specially the second one will be really helpful although will imply a decently-big rework in the Display trait impl.

@CPerezz
Copy link
Contributor Author

CPerezz commented Dec 13, 2022

After 08632e1a the format I achieved for the debug display of VerifyFailure::ConstraintNotSatisfied was as follows:

ConstraintCaseDebug {
            constraint: Constraint {
                gate: Gate {
                    index: 0,
                    name: "Equality check",
                },
                index: 0,
                name: "",
            },
            location: InRegion {
                region: Region 1 ('Wrong synthesis'),
                offset: 0,
            },
            cell_values: [
                (
                    DebugVirtualCell {
                        name: "",
                        column: DebugColumn {
                            column_type: Advice,
                            index: 0,
                            annotation: "This is Advice!",
                        },
                        rotation: 0,
                    },
                    "1",
                ),
                (
                    DebugVirtualCell {
                        name: "",
                        column: DebugColumn {
                            column_type: Advice,
                            index: 1,
                            annotation: "This is Advice too!",
                        },
                        rotation: 0,
                    },
                    "0",
                ),
                (
                    DebugVirtualCell {
                        name: "",
                        column: DebugColumn {
                            column_type: Advice,
                            index: 2,
                            annotation: "Another one!",
                        },
                        rotation: 0,
                    },
                    "0x5",
                ),
                (
                    DebugVirtualCell {
                        name: "",
                        column: DebugColumn {
                            column_type: Fixed,
                            index: 0,
                            annotation: "This is a Fixed!",
                        },
                        rotation: 0,
                    },
                    "0x7",
                ),
            ],
        },

As you can see, this requires auxiliary types which are obfuscated to any other part of the lib but that are necessary in order to be able to inject the Column names inside of the Column section itself.
This also required to re-implement manually Debug and Display for this enum.

CPerezz added a commit to privacy-scaling-explorations/halo2 that referenced this issue Dec 13, 2022
It was necessary to improve the `prover.verify` output also. To do so,
this required auxiliary types which are obfuscated to any other part of the lib
but that are necessary in order to be able to inject the Column names inside of
the `Column` section itself.
This also required to re-implement manually `Debug` and `Display` for this enum.

This closes zcash#705
@therealyingtong
Copy link
Collaborator

This issue is also related to #681.

CPerezz added a commit to privacy-scaling-explorations/halo2 that referenced this issue Jan 10, 2023
It was necessary to improve the `prover.verify` output also. To do so,
this required auxiliary types which are obfuscated to any other part of the lib
but that are necessary in order to be able to inject the Column names inside of
the `Column` section itself.
This also required to re-implement manually `Debug` and `Display` for this enum.

This closes zcash#705
CPerezz added a commit to CPerezz/halo2 that referenced this issue Jan 10, 2023
It was necessary to improve the `prover.verify` output also. To do so,
this required auxiliary types which are obfuscated to any other part of the lib
but that are necessary in order to be able to inject the Column names inside of
the `Column` section itself.
This also required to re-implement manually `Debug` and `Display` for this enum.

This closes zcash#705
CPerezz added a commit to privacy-scaling-explorations/halo2 that referenced this issue Jan 16, 2023
* feat: Add `name_column` to `Layouter` & `RegionLayouter`

This adds the trait-associated function `name_column` in order to enable
the possibility of the Layouter to store annotations aobut the colums.

This function does nothing for all the trait implementors (V1,
SimpleFloor, Assembly....) except for the `MockProver`. Which is
responsible of storing a map that links within a `Region` index, the
`column::Metadata` to the annotation `String`.

* feta: Update metadata/dbg structs to hold Col->Ann mapping

* feat: Update emitter module to print Column annotations

* feat: Add lookup column annotations

This adds the fn `annotate_lookup_column` for `ConstraintSystem` which
allows to carry annotations for the lookup columns declared for a
circuit within a CS.

* feat: Add Lookup TableColumn annotations

This allows to annotate lookup `TableColumn`s and print it's annotation
within the `assert_satisfied` fn.

This has required to change the `ConstraintSystem::lookup_annotations`
to have keys as `metadata::Column` rather than `usize` as otherwise it's
impossible within the `emitter` scope to distinguish between regular
advice columns (local to the Region) and fixed columns which come from
`TableColumn`s.

* fix: Customly derive PartialEq for metadata::Region

This allows to ignore the annotation map of the metadata::Region so that
is easier to match against `VerifyFailure` errors in tests.

* fix: Update ConstraintNotSatisfied testcase

* fix: Update Debug & Display for VerifyFailure

It was necessary to improve the `prover.verify` output also. To do so,
this required auxiliary types which are obfuscated to any other part of the lib
but that are necessary in order to be able to inject the Column names inside of
the `Column` section itself.
This also required to re-implement manually `Debug` and `Display` for this enum.

This closes zcash#705

* fix: Address clippy & warnings

* fix: Add final comments & polish

* fix: Resolve cherry-pick merge conflics & errors

* chore: Change DebugColumn visibility

* chore: Allow to fetch annotations from metadata

* chore: Fix clippy lints

* chore: Remove comments from code for testing

* feat: Add support for Advice and Instance anns in lookups

* feat: Allow `V1` layouter to annotate columns too

* fix: Support `Constant` & `Selector` for lookup exprs

* chore: Address review comments

* chore: Propagete write! result in `VerifyFailure::Display`

* chore: Address clippy lints
jonathanpwang added a commit to axiom-crypto/halo2 that referenced this issue Aug 15, 2023
* - Implements `PartialOrd` for `Value<F>`
- Adds a `transpose` method to turn `Value<Result<_>>` into
  `Result<Value<_>>`
- `Expression::identifier()` remove string memory reallocation

* Fix MockProver `assert_verify` panic errors (privacy-scaling-explorations#118)

* fix: Support dynamic lookups in `MockProver::assert_verify`

Since lookups can only be `Fixed` in Halo2-upstream, we need to add
custom suport for the error rendering of dynamic lookups which doesn't
come by default when we rebase to upstream.

This means that now we have to print not only `AdviceQuery` results to
render the `Expression` that is being looked up. But also support
`Instance`, `Advice`, `Challenge` or any other expression types that are
avaliable.

This addresses the rendering issue, renaming also the `table_columns`
variable for `lookup_columns` as the columns do not have the type
`TableColumn` by default as opposite to what happens upstream.

* fix: Don't error and emit empty String for Empty queries

* feat: Add `assert_sarisfied_par` fn to `MockProver`

* fix: Address clippy errors

* chore: Address review comments

* chore: Fix clippy lints

Resolves: privacy-scaling-explorations#116

* Remove partial ordering for value

* Remove transpose

* Parallelize SHPLONK multi-open prover (privacy-scaling-explorations#114)

* feat: parallelize (cpu) shplonk prover

* shplonk: improve `construct_intermediate_sets` using `BTreeSet` and
`BTreeMap` more aggressively

* shplonk: add `Send` and `Sync` to `Query` trait for more parallelization

* fix: ensure the order of the collection of rotation sets is independent
of the values of the opening points

Co-authored-by: Jonathan Wang <jonathanpwang@users.noreply.github.com>

* fix: FailureLocation::find empty-region handling (privacy-scaling-explorations#121)

After working on fixing
privacy-scaling-explorations/zkevm-circuits#1024, a bug was found in the
verification fn of the MockProver which implies that while finding a
FailureLocation, if a Region doesn't contain any rows.

This is fixed by introducing a 2-line solution suggested by @lispc.

Resolves: privacy-scaling-explorations#117

* Feature: Expose Fixed columns & Assembly permutation structs in MockProver instance (privacy-scaling-explorations#123)

* feat: Expose fixed columns in MockProver

* change: Make `Assembly` object public & add getters

* chore: Address leftover TODOs

* Feature to serialize/deserialize KZG params, verifying key, and proving key into uncompressed Montgomery form (privacy-scaling-explorations#111)

* feat: read `VerifyingKey` and `ProvingKey` does not require `params` as
long as we serialize `params.k()`

* feat: add features "serde-raw" and "raw-unchecked" to
serialize/deserialize KZG params, verifying key, and proving key
directly into raw bytes in internal memory format.
So field elements are stored in Montgomery form `a * R (mod p)` and
curve points are stored without compression.

* chore: switch to halo2curves 0.3.1 tag

* feat: add enum `SerdeFormat` for user to select
serialization/deserialization format of curve and field elements

Co-authored-by: Jonathan Wang <jonathanpwang@users.noreply.github.com>

* Add support for Column annotations for MockProver debugging (privacy-scaling-explorations#109)

* feat: Add `name_column` to `Layouter` & `RegionLayouter`

This adds the trait-associated function `name_column` in order to enable
the possibility of the Layouter to store annotations aobut the colums.

This function does nothing for all the trait implementors (V1,
SimpleFloor, Assembly....) except for the `MockProver`. Which is
responsible of storing a map that links within a `Region` index, the
`column::Metadata` to the annotation `String`.

* feta: Update metadata/dbg structs to hold Col->Ann mapping

* feat: Update emitter module to print Column annotations

* feat: Add lookup column annotations

This adds the fn `annotate_lookup_column` for `ConstraintSystem` which
allows to carry annotations for the lookup columns declared for a
circuit within a CS.

* feat: Add Lookup TableColumn annotations

This allows to annotate lookup `TableColumn`s and print it's annotation
within the `assert_satisfied` fn.

This has required to change the `ConstraintSystem::lookup_annotations`
to have keys as `metadata::Column` rather than `usize` as otherwise it's
impossible within the `emitter` scope to distinguish between regular
advice columns (local to the Region) and fixed columns which come from
`TableColumn`s.

* fix: Customly derive PartialEq for metadata::Region

This allows to ignore the annotation map of the metadata::Region so that
is easier to match against `VerifyFailure` errors in tests.

* fix: Update ConstraintNotSatisfied testcase

* fix: Update Debug & Display for VerifyFailure

It was necessary to improve the `prover.verify` output also. To do so,
this required auxiliary types which are obfuscated to any other part of the lib
but that are necessary in order to be able to inject the Column names inside of
the `Column` section itself.
This also required to re-implement manually `Debug` and `Display` for this enum.

This closes zcash#705

* fix: Address clippy & warnings

* fix: Add final comments & polish

* fix: Resolve cherry-pick merge conflics & errors

* chore: Change DebugColumn visibility

* chore: Allow to fetch annotations from metadata

* chore: Fix clippy lints

* chore: Remove comments from code for testing

* feat: Add support for Advice and Instance anns in lookups

* feat: Allow `V1` layouter to annotate columns too

* fix: Support `Constant` & `Selector` for lookup exprs

* chore: Address review comments

* chore: Propagete write! result in `VerifyFailure::Display`

* chore: Address clippy lints

* chore: Move Codecov, wasm-build, Bitrot & doc-tests to push (privacy-scaling-explorations#125)

* chore: Move Codecov, wasm-build, Bitrot & doc-tests to push

This should cut down significantly the CI times on every push done to a
branch for a PR.
Resolves: privacy-scaling-explorations#124

* chore: Add back `push` on CI checks

* fix: Allow to compare `Assembly` structs (privacy-scaling-explorations#126)

This was missing in privacy-scaling-explorations#123 so this PR fixes it.

* Add keccak256 hasher for transcript (#2)

* Add keccak256 hasher for transcript

* Fix keccak256 common point prefix

* Remove unnecessary hasher_* variables

* fix: transcript instantiation in poseidon benchmark loop (privacy-scaling-explorations#128)

* Improve performance of vk & pk keygen and of default `parallelize` chunking size (privacy-scaling-explorations#127)

* Squashed commit of the following:

commit 17e3c4e
Author: Mickey <hedgefund996@gmail.com>
Date:   Fri Jul 15 11:10:32 2022 +0800

    speed up generate vk pk with multi-thread

* fix

* Improve performance of vk & pk keygen and of default `parallelize` chunking size.

Reduces proving time on large circuits consistently >=3%.
Builts upon [speed up generate vk pk with multi-thread](privacy-scaling-explorations#88)
Fixes: privacy-scaling-explorations#83

* fix: Force `VerifyFailure` to own the annotations map (privacy-scaling-explorations#131)

* fix: Force `VerifyFailure` to own the annotations map

Since otherwise we can't move the `VerifyFailure` vec's confortably, and
also, we're required to have a lot of lifetime annotations, it was
decided to force the `VerifyFailure` to own the Annotation maps.

This shouldn't be too harmful as it only triggers when testing.

Resolves: privacy-scaling-explorations#130

* chore: Address clippy lints

* feat: call synthesize in `MockProver` multiple times to behave same as real prover

* feat: check advice assignment consistency between different phases

* fix: Support annotations for CellNotAssigned in verify_par (privacy-scaling-explorations#138)

* feat: Add `assert_satisfied_at_rows_par` variant (privacy-scaling-explorations#139)

Resolves: privacy-scaling-explorations#133

* Expose mod `permutation` and re-export `permutation::keygen::Assembly` (privacy-scaling-explorations#149)

* feat: expose mod ule `permutation` and re-export `permutation::keygen::Assembly`

* feat: derive `lone` for `permutation::keygen::Assembly`

* feat: bump MSRV for `inferno`

* feat(MockProver): replace errors by asserts

In MockProver, replace all code that returns an error by an assert that panics instead of returning the error.  This change aims to make it easier to debug circuit code bugs by getting backtraces.

* MockProver test utililities (privacy-scaling-explorations#153)

* test/unwrap_value: escape Value safety in the dev module

* test/mock-prover-values: MockProver exposes the generated columns to tests

* test/mock-prover-values: doc

* mockprover-util: remove unwrap_value

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* feat: Parallel random blinder poly impl (privacy-scaling-explorations#152)

* feat: Parallelize `commit` blinder poly generator method

Solves the concerns raised in privacy-scaling-explorations#151 related to the performance of the
random poly generator inside of `commit`.

Resolves: privacy-scaling-explorations#151

* chore: add `from_evals` for Polynomial

* chore: add benches for commit_zk serial vs par

* fix: Correct thread_seeds iter size

* fix: Clippy

* chore: apply review suggestions

* fix: Inconsisten num of Scalars generated parallely

This fix from @ed255 fixes an error on the code proposal which was
rounding the num of Scalars to be generated and so, was producing
failures.

Co-authored-by: Edu <eduardsanou@posteo.net>

* remove: legacy comments & code

---------

Co-authored-by: Edu <eduardsanou@posteo.net>

* change: Migrate workspace to pasta_curves-0.5 (privacy-scaling-explorations#157)

* change: Migrate workspace to pasta_curves-0.5

This ports the majority of the workspace to the `pasta_curves-0.5.0`
leaving some tricky edge-cases that we need to handle carefully.

Resolves: privacy-scaling-explorations#132

* fix: Complete latest trait bounds to compile halo2proofs

* change: Migrate examples & benches to pasta 0.5

* change: Migrate halo2_gadgets to pasta-0.5

* change: Update gadgets outdated code with latest upstream

* fix: Sha3 gadget circuit

* fix: doc tests

* chore: Update merged main

* fix: Apply review suggestions

* fix: pin `halo2curves` version to `0.3.2`

* Extend Circuit trait to take parameters in config (privacy-scaling-explorations#168)

* Extend Circuit trait to take parameters in config

The Circuit trait is extended with the following:
```
pub trait Circuit<F: Field> {
    /// [...]
    type Params: Default;

    fn params(&self) -> Self::Params {
        Self::Params::default()
    }

    fn configure_with_params(meta: &mut ConstraintSystem<F>, params: &Self::Params) -> Self::Config {
        Self::configure(meta)
    }

    fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config;
}
```

This allows runtime parametrization of the circuit configuration.  The extension to the Circuit trait has been designed to minimize the breaking change: existing circuits only need to define the associated `type Params`.

Unfortunately "Associated type defaults" are unstable in Rust, otherwise this would be a non-breaking change.  See rust-lang/rust#29661

* Implement circuit params under feature flag

* Don't overwrite configure method

* Fix doc test

* Allow halo2 constraint names to have non static names (privacy-scaling-explorations#156)

* static ref to String type in Gates, Constraints, VirtualCell, Argument

* 'lookup'.to_string()

* return &str for gate name and constriant_name, also run fmt

* Update halo2_gadgets/Cargo.toml

Co-authored-by: Han <tinghan0110@gmail.com>

* upgrade rust-toochain

---------

Co-authored-by: Carlos Pérez <37264926+CPerezz@users.noreply.github.com>
Co-authored-by: Han <tinghan0110@gmail.com>

* Improve halo2 query calls (privacy-scaling-explorations#154)

* return expression from cell

* add example

* selector

* recurse Expression to fill in index

* minimized changes from the original

* backword compatible meta.query_X & challange.expr()

* cargo fmt

* fixed lookup to pass all tests

* Update comments

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

* Update comments

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

* Update comments

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

* Update comments

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

* Update comments

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

* Update comments

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

* update

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

* add primitives.rs back

* remove example2

* backward compatible meta.query_X & Column.cur(), next(), prev(), at(usize)

* impl Debug & make side effects only when query.index.is_none()

* change impl Debug for Expression instead & revert test in plonk_api

* upgrade rust-toolchain

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <tinghan0110@gmail.com>

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <tinghan0110@gmail.com>

* ran clippy

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <tinghan0110@gmail.com>

---------

Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>
Co-authored-by: Han <tinghan0110@gmail.com>

* fix: compute `num_chunks` more precisely (privacy-scaling-explorations#172)

* Implement Clone trait for Hash, Absorbing, and Sponge structs (privacy-scaling-explorations#171)

* Revert double-assignment mock prover check

Revert the check introduced in
privacy-scaling-explorations#129 to detect double
assignments with different values, because it breaks some tests in the zkevm
project.

There's a legitimate use case of double assignment with different values, which
is overwriting cells in order to perform negative tests (tests with bad witness
that should not pass the constraints).

Also in the EVM Circuit from the zkevm project we "abuse" the assignment of
cells as a cache: sometimes we assign some cells with a guess value, and later
on we reassign with the correct value.

I believe this check is interesting to have, so we could think of ways to add
it back as an optional feature.

* fix: Fix serialization for VerifyingKey (privacy-scaling-explorations#178)

Now the value returned when the number of selectors is a multiple of 8
is correct.

Resolves: privacy-scaling-explorations#175

* Add more getters to expose internal fields

* add a constructor (privacy-scaling-explorations#164)

* add a constructor

* add more comment

* fix as review

* remove clone

* remove

* no need to use new variable

* change comment

* fix clippy

* rename to from_parts

* remove n declaration

* feat: send sync region (privacy-scaling-explorations#180)

* feat: send / sync region

* Update layout.rs

* update

* lol

* debug

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* thread-safe-region feature flag

* cleanup

* patch dev-graph

* patch non-determinism in mapping creation

* reduce mem usage for vk and pk

* mock proving examples

* swap for hashmap for insertion speed

* reduce update overhead

* replace BTree with Vec

* add benchmarks

* make the benchmarks massive

* patch clippy

* simplify lifetimes

* patch benches

* Update halo2_proofs/src/plonk/permutation/keygen.rs

Co-authored-by: Han <tinghan0110@gmail.com>

* Update halo2_proofs/examples/vector-mul.rs

Co-authored-by: Han <tinghan0110@gmail.com>

* rm benches

* order once

* patch lints

---------

Co-authored-by: Han <tinghan0110@gmail.com>

* Fix `parallelize` workload imbalance (privacy-scaling-explorations#186)

* fix parallelize workload imbalance

* remove the need of unsafe

* implement native shuffle argument and api

* fix: remove nonsense comment

* strictly check shuffle rows

* address doc typos

* move compression into product commitment

* typo

* add shuffle errors for `verify_at_rows_par`

* dedup expression evaluation

* cargo fmt

* fix fields in sanity-checks feature

* Updates halo2_curves dependency to released package (privacy-scaling-explorations#190)

THe package release ressets the version from those inherited by the legacy
halo2curves repo's fork history.

The upstream diff is:
https://github.com/privacy-scaling-explorations/halo2curves/compare/9f5c50810bbefe779ee5cf1d852b2fe85dc35d5e..9a7f726fa74c8765bc7cdab11519cf285d169ecf

* chore: remove monorepo

Go back to having halo2curves and poseidon in separate repos.

* chore: fix clippy and tests

* fix: remove thread-safe-regions feature

`WitnessCollection` in `create_proof` isn't thread-safe.
We removed `Region`s from `SimpleLayouter` anyways.

* fix: rustfmt

* fix: dev-graph

* chore: update lint CI name

* chore: fix clippy

* chore: autoexample = false

turn off examples that use layouter

* chore(CI): separate job for examples

* chore: remove prefetch from asm, not used

* chore: fix asm feature

---------

Co-authored-by: adria0 <nowhere@>
Co-authored-by: Carlos Pérez <37264926+CPerezz@users.noreply.github.com>
Co-authored-by: adria0.eth <5526331+adria0@users.noreply.github.com>
Co-authored-by: Jonathan Wang <jonathanpwang@users.noreply.github.com>
Co-authored-by: kilic <kiliconu@itu.edu.tr>
Co-authored-by: dante <45801863+alexander-camuto@users.noreply.github.com>
Co-authored-by: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Co-authored-by: han0110 <tinghan0110@gmail.com>
Co-authored-by: Eduard S <eduardsanou@posteo.net>
Co-authored-by: naure <naure@users.noreply.github.com>
Co-authored-by: Aurélien Nicolas <info@nau.re>
Co-authored-by: CeciliaZ030 <45245961+CeciliaZ030@users.noreply.github.com>
Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>
Co-authored-by: Enrico Bottazzi <85900164+enricobottazzi@users.noreply.github.com>
Co-authored-by: Ethan-000 <s2026080@ed.ac.uk>
Co-authored-by: Mamy Ratsimbazafy <mamy_github@numforge.co>
Co-authored-by: kilic <onurkilic1004@gmail.com>
Co-authored-by: François Garillot <4142+huitseeker@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dev-tooling Area: Developer tooling A-errors Area: Error handling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants