-
Notifications
You must be signed in to change notification settings - Fork 231
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
feat(zk): implement combined custom gate column builder #206
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
dongchangYoo
force-pushed
the
feat/implement-circuit-poly-builder
branch
from
December 18, 2023 08:23
1d71413
to
f678dc0
Compare
d19cf3f This should be |
chokobole
reviewed
Dec 18, 2023
dongchangYoo
force-pushed
the
feat/implement-circuit-poly-builder
branch
5 times, most recently
from
December 20, 2023 06:22
cdd1ba8
to
21233b6
Compare
chokobole
reviewed
Dec 21, 2023
Insun35
reviewed
Dec 21, 2023
fakedev9999
requested changes
Dec 21, 2023
dongchangYoo
force-pushed
the
feat/implement-circuit-poly-builder
branch
from
December 22, 2023 03:59
21233b6
to
dcf1a0c
Compare
`VanishingArgument` actually only requires `Field` type as a template argument.
…y loop An issue arises in the `VanishingArgument::Create()` where the destructor of `ValueSource` is called infinitely during the process of adding `Calculation::HornerData` to `custom_gates`. This issue is suspected to stem from managing the members of `Calculation` as a `Union`, hence each member was changed to `std::optional`.
We switched it to public for external logic that uses only this feature, and changed it to `static` since this function does not use any internal variables.
See [CoeffToExtendedPart](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/poly/domain.rs#L341-L355) and [BuildExtendedColumnWithColumns](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/poly/domain.rs#L171-L186)
`Table` does not own the data of its members, so rename it to `RefTable` to improve the user's intuitive understanding.
dongchangYoo
force-pushed
the
feat/implement-circuit-poly-builder
branch
from
December 22, 2023 04:10
dcf1a0c
to
9837c2a
Compare
fakedev9999
approved these changes
Dec 22, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Insun35
approved these changes
Dec 22, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In the next commit, `OwnedTable` will be implemented with an interface same to `RefTable`, but it owns data of members. Therefore the common methods of two child classes was implemented in `TableBase`.
It owns `Fixed/Advice/Instance` columns and is a class that returns a `Ref` to the respective column when given a `ColumnKey` as input. It is similar to Table in `zk/plonk/circuit/table.h`.
It is a class that manages the arguments of `ValueSource::Get()`, `Calculation::Evaluate()`, and `GraphEvaluator::Evaluate()` together. It will replace `ValueSourceData` on the next commit.
It generates circuit polynomial: gate₀(X) + y * gate₁(X) + ... + yⁱ * gateᵢ(X) + ... See [Halo2 book](https://zcash.github.io/halo2/design/proving-system/vanishing.html) And To find reference of this log, see [evaluate_h()](See [evaluate_h()](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/plonk/evaluation.rs#L280-L583)) And this feature is provided by the `VanishingArgument::BuildExtendedCircuitColumn()` function.
dongchangYoo
force-pushed
the
feat/implement-circuit-poly-builder
branch
from
December 22, 2023 05:55
9837c2a
to
a9f048b
Compare
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.
This PR mainly implements a function named
BuildCombinedCustomGateColumn()
.And it corresponds to evaluate_h() in
Zcash
implementations.Additionally, some bugs in the
VanishingArgument
class have been fixed, and the process also includes assigning aVanishingArgument
instance to theProvingKey
.