Skip to content

chore: stdlib bool internal audit #15070

Merged
iakovenkos merged 20 commits into
merge-train/barretenbergfrom
si/stdlib-bool-internal-audit-0
Jul 4, 2025
Merged

chore: stdlib bool internal audit #15070
iakovenkos merged 20 commits into
merge-train/barretenbergfrom
si/stdlib-bool-internal-audit-0

Conversation

@iakovenkos

@iakovenkos iakovenkos commented Jun 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

  • Our basic binary ops produce a new gate iff both operands are witnesses. Most of the implementations contained deeply branched logic that is now replaced with simple int arithmetic that looks more robust and is much easier to read.

  • Added documentation in various places. The constraints are made more explicit

  • Removed redundant assignments

  • Re-worked the test suite. Now all basic ops and functions are tested for all combinations of flags (is_constant, witness_bool, witness_inverted). Added missing tests, checked various edge cases.

  • Removed a couple of tests that seemed less expressive

  • Removed must_imply() method for a vector of implications, as it is not used anymore

  • Fixed a normalize() regression - previously it wouldn't check if (witness_inverted == false) and would normalize a witness bool_t even if the condition is satisfied. It is the reason for the VK changes

iakovenkos
iakovenkos commented Jun 30, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp
iakovenkos
iakovenkos commented Jun 30, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp
iakovenkos
iakovenkos commented Jun 30, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.test.cpp
iakovenkos
iakovenkos commented Jun 30, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.test.cpp
iakovenkos
iakovenkos commented Jun 30, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.test.cpp
iakovenkos
iakovenkos commented Jun 30, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp
ASSERT(result == true);
}

bool_t acc = witness_t(ctx, true); // will accumulate the conjunctions of each condition (i.e. `&&` of each)

@iakovenkos iakovenkos Jun 30, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unconstrained witness, replaced with a constant true, it'll be turned into a witness after && with the first witness

@iakovenkos iakovenkos self-assigned this Jul 1, 2025
@iakovenkos iakovenkos changed the title chore: stdlib bool internal audit pt.0 chore: stdlib bool internal audit Jul 1, 2025
@iakovenkos iakovenkos requested a review from suyash67 July 1, 2025 12:04
suyash67
suyash67 reviewed Jul 2, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp Outdated
suyash67
suyash67 reviewed Jul 2, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp Outdated
suyash67
suyash67 reviewed Jul 2, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp
suyash67
suyash67 reviewed Jul 2, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp
suyash67
suyash67 reviewed Jul 2, 2025
View reviewed changes
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp
/**
* @brief Implements implication operator in circuit.
*/
template <typename Builder> bool_t<Builder> bool_t<Builder>::implies(const bool_t<Builder>& other) const

@suyash67 suyash67 Jul 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implies, must_imply and implies_both_ways were added for aztec connect if I remember correctly. If these are not being used anywhere (including noir), I am in favour of removing them. Ideally, I think bool class should only contain standard operations (!, |, ^, &).

@suyash67 suyash67 Jul 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think only must_imply is currently being used in ecdsa verifier and some places in recursive verifier.

@iakovenkos iakovenkos marked this pull request as ready for review July 3, 2025 16:05
@iakovenkos iakovenkos requested a review from LeilaWang as a code owner July 3, 2025 16:05
@iakovenkos iakovenkos marked this pull request as draft July 3, 2025 16:07
@iakovenkos iakovenkos marked this pull request as ready for review July 3, 2025 16:12
@iakovenkos iakovenkos changed the base branch from next to merge-train/barretenberg July 4, 2025 10:45
@iakovenkos iakovenkos merged commit e261704 into merge-train/barretenberg Jul 4, 2025
5 checks passed
@iakovenkos iakovenkos deleted the si/stdlib-bool-internal-audit-0 branch July 4, 2025 10:45
@AztecBot AztecBot mentioned this pull request Jul 4, 2025
Merged
github-merge-queue Bot pushed a commit that referenced this pull request Jul 4, 2025
@ludamad @iakovenkos
@federicobarbacovi @suyash67 @johnathan79717 @ledwards2225
feat: merge-train/barretenberg (#15509)
bd82c68 
See
[merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md).

chore: stdlib bool internal audit  (#15070)
feat: improve Shplonk api (#15422)
fix(merge-train): don't queue merge if merge-train failed queue, pass on
rebase logic (#15508)
chore: nuke bit array (#15522)
chore: remove template parameters (#15530)

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com>
@AztecBot AztecBot mentioned this pull request Jul 8, 2025
Merged
github-merge-queue Bot pushed a commit that referenced this pull request Jul 9, 2025
@ludamad @iakovenkos
@federicobarbacovi @suyash67 @johnathan79717 @ledwards2225 @notnotraju @lucasxia01 @kashbrti @jeanmon @alexghr @spalladino
feat: merge-train/barretenberg (#15586)
c33758f 
See
[merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md).

BEGIN_COMMIT_OVERRIDE
chore: stdlib bool internal audit  (#15070)
feat: improve Shplonk api (#15422)
fix(merge-train): don't queue merge if merge-train failed queue, pass on
rebase logic (#15508)
chore: nuke bit array (#15522)
chore: remove template parameters (#15530)
chore: no PK (#15386)
chore!: Correct public inputs propagation in the tube (#15547)
chore: use `batch_invert` in native IPA verifier (#15557)
chore: Move `stdlib::uint_plookup` to `stdlib::uint` (#15460)
chore: use const ref commitment keys (#15584)
fix: hiding circuit vk computed only once (#15589)
feat: transcript can hash objects independently (#15510)
chore: readme for benchmarking remotely (#15512)
chore: fix avm test (#15592)
chore: hash more stuff for IPA. (#15519)
chore: fix avm build in merge-train/bb (#15594)
feat!: structured public inputs via kernel io (#15383)
fix!: aggregate correct nested pairing points in the hiding circuit
(#15598)
fix: bb merge-train conflicts (#15617)
chore: Refactor shplonk verifier api (#15618)
chore!: databus consistency checks in the hiding circuit (#15599)
feat!: VK hash consistency check (#15591)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com>
Co-authored-by: Raju Krishnamoorthy <krishnamoorthy@gmail.com>
Co-authored-by: notnotraju <raju@aztec-labs.com>
Co-authored-by: Lucas Xia <lucasxia01@gmail.com>
Co-authored-by: Khashayar Barooti <khashayar@aztecprotocol.com>
Co-authored-by: Jean M <132435771+jeanmon@users.noreply.github.com>
Co-authored-by: Alex Gherghisan <alexghr@users.noreply.github.com>
Co-authored-by: Santiago Palladino <spalladino@users.noreply.github.com>
Co-authored-by: Santiago Palladino <santiago@aztec-labs.com>
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jul 16, 2025
@ludamad @iakovenkos
@federicobarbacovi @suyash67 @johnathan79717 @ledwards2225
feat: merge-train/barretenberg (AztecProtocol#15509)
d02b0dc 
See
[merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md).

chore: stdlib bool internal audit  (AztecProtocol#15070)
feat: improve Shplonk api (AztecProtocol#15422)
fix(merge-train): don't queue merge if merge-train failed queue, pass on
rebase logic (AztecProtocol#15508)
chore: nuke bit array (AztecProtocol#15522)
chore: remove template parameters (AztecProtocol#15530)

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@suyash67 suyash67 suyash67 approved these changes

@LeilaWang LeilaWang Awaiting requested review from LeilaWang

Assignees

@iakovenkos iakovenkos

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iakovenkos @suyash67 @ledwards2225