chore: bool_t from witness index functionality #16688
Merged
Merged
Conversation
iakovenkos
commented
Sep 1, 2025
| builder.fix_witness(input.result_infinite, infinite.get_value()); | ||
| } else { | ||
| builder.assert_equal(infinite.witness_index, input.result_infinite); | ||
| builder.assert_equal(infinite.get_normalized_witness_index(), input.result_infinite); |
Contributor
Author
There was a problem hiding this comment.
It's safer to always access the normalized index.
iakovenkos
commented
Sep 1, 2025
| bit.witness_bool = false; | ||
| ctx->create_new_range_constraint( | ||
| bit.witness_index, 1, "biggroup_nafs: compute_naf extracted too many bits in next_entry case"); | ||
| bool_ct bit(witness_t<C>(ctx, !next_entry)); |
Contributor
Author
There was a problem hiding this comment.
use the appropriate constructor to create a constrained bool witness
iakovenkos
commented
Sep 1, 2025
| * @warning The witness value **is not** constrained to be boolean. We simply perform an out-of-circuit sanity check. | ||
| */ | ||
| template <typename Builder> | ||
| bool_t<Builder> bool_t<Builder>::from_witness_index_unsafe(Builder* ctx, const uint32_t witness_index) |
Contributor
Author
There was a problem hiding this comment.
@guipublic here's the promissed functionality. It feels safer to me if all from_witness_index calls would be marked as unsafe, so that it's clear that such calls have to be justified + analyzed more carefully during the audit
for @suyash67 - Luke has spotted that in #16663, the witness index is set up directly to avoid creating an extra boolean gate which is redundant in this case (see the comments there). so I added this method + turned all class variables private
iakovenkos
commented
Sep 1, 2025
| bool get_value() const { return witness_bool ^ witness_inverted; } | ||
|
|
||
| bool is_constant() const { return witness_index == IS_CONSTANT; } | ||
| bool is_inverted() const |
Contributor
Author
There was a problem hiding this comment.
seems cleaner than adding a bunch of friend classes as friends
suyash67
reviewed
Sep 1, 2025
suyash67
reviewed
Sep 1, 2025
…/aztec-packages into si/bool-t-from-witness-idx
iakovenkos
added
the
bberg-int-audit
suyash67
reviewed
Sep 4, 2025
|
|
||
| if (use_range_constraint) { | ||
| // Create a range constraint gate | ||
| context->create_new_range_constraint(witness_index, 3, "bool_t: witness value is not 0 or 1"); |
Contributor
There was a problem hiding this comment.
Should the range be [0, 1] and hence create_new_range_constraint(witness_index, 1, ...)?
…i/bool-t-from-witness-idx
suyash67
approved these changes
Sep 4, 2025
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Sep 4, 2025
BEGIN_COMMIT_OVERRIDE chore: compute beta products for max log dyadic size instead of 21 (#16772) chore: bool_t from witness index functionality (#16688) chore: audit the set relation of the ECCVM (#16706) chore: update readme in the eccvm to include description of multiset and lookups (#16765) END_COMMIT_OVERRIDE
Maddiaa0
pushed a commit
that referenced
this pull request
Sep 4, 2025
Several `bool_t` improvements: * added a `from_witness_index_unsafe` constructor that implicitly appeared in #16663 * turned `witness_index` and other class members private * added a more efficient constructor that takes a `witness_t` and uses range constraints instead of `bool_gate`. see [related issue](AztecProtocol/barretenberg#1533) + the tests As a byproduct: * some simplifications in `compute_naf` method * using `get_normalized_witness_index` on `bool_t` outside of the friend `field_t` and `biggroup` classes * slightly improved `used_witnesses` handling in a couple of places. The vks and the number of gates have changed due to a tiny opt related to re-using a couple of `bool_t` results in `+` and `-` in `biggroup`
mralj
pushed a commit
that referenced
this pull request
Oct 13, 2025
Several `bool_t` improvements: * added a `from_witness_index_unsafe` constructor that implicitly appeared in #16663 * turned `witness_index` and other class members private * added a more efficient constructor that takes a `witness_t` and uses range constraints instead of `bool_gate`. see [related issue](AztecProtocol/barretenberg#1533) + the tests As a byproduct: * some simplifications in `compute_naf` method * using `get_normalized_witness_index` on `bool_t` outside of the friend `field_t` and `biggroup` classes * slightly improved `used_witnesses` handling in a couple of places. The vks and the number of gates have changed due to a tiny opt related to re-using a couple of `bool_t` results in `+` and `-` in `biggroup`
ludamad
pushed a commit
that referenced
this pull request
Dec 16, 2025
Several `bool_t` improvements: * added a `from_witness_index_unsafe` constructor that implicitly appeared in #16663 * turned `witness_index` and other class members private * added a more efficient constructor that takes a `witness_t` and uses range constraints instead of `bool_gate`. see [related issue](AztecProtocol/barretenberg#1533) + the tests As a byproduct: * some simplifications in `compute_naf` method * using `get_normalized_witness_index` on `bool_t` outside of the friend `field_t` and `biggroup` classes * slightly improved `used_witnesses` handling in a couple of places. The vks and the number of gates have changed due to a tiny opt related to re-using a couple of `bool_t` results in `+` and `-` in `biggroup`
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Several
bool_timprovements:from_witness_index_unsafeconstructor that implicitly appeared in chore: use the bb predicates when generating constraints #16663witness_indexand other class members privatewitness_tand uses range constraints instead ofbool_gate. see related issue + the testsAs a byproduct:
compute_nafmethodget_normalized_witness_indexonbool_toutside of the friendfield_tandbiggroupclassesused_witnesseshandling in a couple of places.The vks and the number of gates have changed due to a tiny opt related to re-using a couple of
bool_tresults in+and-inbiggroup