chore: use the bb predicates when generating constraints by guipublic · Pull Request #16663 · AztecProtocol/aztec-packages

guipublic · 2025-08-29T16:18:34Z

This PR adds support for using the acir predicates for some black-box function calls on barretenberg side

For recursive proving, we conditionally assign the inputs when the predicate is false to a placeholder verifiable proof and vk
For elliptic curve points, we conditionally assign the inputs when the predicate is false, to valid dummy values

ecdsa r1 is not done on purpose (it'll be merged into one ecdsa file)

iakovenkos · 2025-08-29T17:40:54Z

+    }
+    // creates a bool_ct from the index, without adding a boolean gate because a predicate is boolean by definition.
+    FF predicate_value = builder.get_variable(predicate.index);
+    bool_ct predicate_witness = bool_ct(&builder, predicate_value == FF(1));


I don't have enough context here, but it seems a bit suspicious that any predicate_value except for 1 is getting converted to 0. Can you please explain why it's safe?

Is it because inside of the to_grumkin_point(...) we have
auto infinite = bool_ct(to_field_ct(input_infinite, builder));?
Should we maybe add a from_bool_witness_index in bool_t?

The predicate is coming from Noir, as a condition guarding a conditional branch.
For instance, if you have:
if x ==3 { C = ec_add(A,B); }, the predicate will be x==3, which is a boolean value. When it is computed by Noir, it is properly constrained to be boolean.

Should we maybe add a from_bool_witness_index in bool_t?

Yes that could be useful for this case.

Thanks for the explanation! I'm spinning up a PR to add this constructor

my PR got merged to next, so you can use from_witness_undex_unsafe() on bool_t

TomAFrench · 2025-09-04T13:25:53Z

@guipublic looks like this has been affected by changes the crypto team has made to how we handle ECDSA opcodes. Can you update this please?

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`

ledwards2225 · 2025-09-05T03:57:00Z

@guipublic hope this hasn't been waiting on me - I got the sense that it wasn't ready for review yet. Lmk!

TomAFrench · 2025-09-05T12:06:04Z

Nope, don't worry. This has been semi-blocked by us getting the acir serialization PR in until recently.

ledwards2225

Overall these changes seem ok to me but I think there's still some work to do to get this to a merge-able state. My main concerns are

Ensuring all of this new functionality is fully tested
Making sure @iakovenkos and @federicobarbacovi are happy with and aware of the changes in bool / ecdsa respectively
Making sure the complex portions (especially of verifier input mocking) are isolated, clean and well documented

ledwards2225 · 2025-09-30T19:03:22Z

    Fq pub_x(pub_x_bytes);
    Fq pub_y(pub_y_bytes);
    G1 public_key(pub_x, pub_y);
+    // Update it depending on the predicate


Would be good to have @federicobarbacovi take a quick look since this code has been audited

ledwards2225 · 2025-09-30T19:44:49Z

@@ -117,7 +117,6 @@ TEST(Databus, UnnormalizedEntryAccess)
    std::vector<field_ct> calldata_entries;
    for (fr entry : raw_calldata_entries) {
        calldata_entries.emplace_back(witness_ct(&builder, entry));


unrelated change?

This line is giving me compilation error because entry_witness is not used.

federicobarbacovi · 2025-10-03T11:05:44Z

    Fq pub_y(pub_y_bytes);
    G1 public_key(pub_x, pub_y);
+    // Update it depending on the predicate
+    if (!input.predicate.is_constant) {


Suggested change

if (!input.predicate.is_constant) {

if (!input.predicate.is_constant) {

// We need to use a point which is different from the generator otherwise secp256r1 ECDSA verification fails

auto default_point = Curve::g1::one + Curve::g1::one;

bool_ct predicate_witness =

bool_ct::from_witness_index_unsafe(&builder, input.predicate.index);

pub_x = Fq::conditional_assign(predicate_witness, pub_x, default_point.x);

pub_y = Fq::conditional_assign(predicate_witness, pub_y, default_point.y);

} else {

BB_ASSERT_EQ(input.predicate.value, true, "Creating ECDSA constraints with a constant predicate equal to false.");

}

conditional_assign works in reverse respect to conditional_select, I think the code as written above might be easier to read. Also, we need to use a point which is not ± the generator to avoid issues with secp256r1

Could you please also add documentation regarding this change in the doxygen documentation for the function create_ecdsa_verify_constraints, and documentation regarding predicate to the doxygen documentation of the class EcdsaConstraint? Thanks!

Finally, I think it would be good to add some tests in ecdsa_constraints.test.cpp testing that when we supply the predicate things work as expected.

ledwards2225

LGTM, thanks for making updates!

ledwards2225 · 2025-10-09T14:39:19Z

+    // constuct a circuit with a single gate
+    Builder builder;
+
+    // Create 2^log_n many add gates based on input log num gates


flagging this again

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`

TomAFrench

LGTM

This PR adds support for using the acir predicates for some black-box function calls on barretenberg side - For recursive proving, we conditionally assign the inputs when the predicate is false to a placeholder verifiable proof and vk - For elliptic curve points, we conditionally assign the inputs when the predicate is false, to valid dummy values ecdsa r1 is not done on purpose (it'll be merged into one ecdsa file) Co-authored-by: Alejo Amiras <alejo.amiras@gmail.com> Co-authored-by: Alex Gherghisan <alexghr@users.noreply.github.com> Co-authored-by: Ary Borenszweig <asterite@gmail.com> Co-authored-by: Charlie <5764343+charlielye@users.noreply.github.com> Co-authored-by: Charlie Lye <5764343+charlielye@users.noreply.github.com> Co-authored-by: DanielKotov <159419107+DanielKotov@users.noreply.github.com> Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com> Co-authored-by: Esau <esau@aztecprotocol.com> Co-authored-by: Facundo <fcarreiro@users.noreply.github.com> Co-authored-by: Gregorio Juliana <gregojquiros@gmail.com> Co-authored-by: Harsh Bajpai <bajpaiharsh244@gmail.com> Co-authored-by: Ilyas Ridhuan <ilyas@aztecprotocol.com> Co-authored-by: IlyasRidhuan <ilyasridhuan@gmail.com> Co-authored-by: Innokentii Sennovskii <isennovskiy@gmail.com> Co-authored-by: James Zaki <james.zaki@proton.me> Co-authored-by: Jan Beneš <janbenes1234@gmail.com> Co-authored-by: Jean M <132435771+jeanmon@users.noreply.github.com> Co-authored-by: Jonathan Hao <johnathan79717@gmail.com> Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com> Co-authored-by: Jonathan Hao <jonathanpohsianghao@gmail.com> Co-authored-by: Josh Crites <jc@joshcrites.com> Co-authored-by: José Pedro Sousa <jose@aztecprotocol.com> Co-authored-by: José Pedro Sousa <outgoing@zpedro.dev> Co-authored-by: Khashayar Barooti <khashayar@aztecprotocol.com> Co-authored-by: LHerskind <16536249+LHerskind@users.noreply.github.com> Co-authored-by: Lasse Herskind <16536249+LHerskind@users.noreply.github.com> Co-authored-by: Leila Wang <leizciw@gmail.com> Co-authored-by: Lin Oshitani <linoshitani@gmail.com> Co-authored-by: Lucas Xia <lucasxia01@gmail.com> Co-authored-by: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Co-authored-by: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Co-authored-by: Michael Connor <mike@aztecprotocol.com> Co-authored-by: Miranda Wood <miranda@aztecprotocol.com> Co-authored-by: MirandaWood <miranda@aztecprotocol.com> Co-authored-by: Mitch <mitchell@aztecprotocol.com> Co-authored-by: Mitchell Tracy <mitchellftracy@gmail.com> Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com> Co-authored-by: Paperclip Minimizer <minim@wonderland.xyz> Co-authored-by: PhilWindle <60546371+PhilWindle@users.noreply.github.com> Co-authored-by: PhilWindle <philip.windle@gmail.com> Co-authored-by: Ragnar <rodiondenmark@gmail.com> Co-authored-by: Rahul Kothari <rahul.kothari.201@gmail.com> Co-authored-by: Raju Krishnamoorthy <krishnamoorthy@gmail.com> Co-authored-by: Rumata888 <isennovskiy@gmail.com> Co-authored-by: Santiago Palladino <santiago@aztec-labs.com> Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com> Co-authored-by: Sarkoxed <75146596+Sarkoxed@users.noreply.github.com> Co-authored-by: Sarkoxed <sarkoxed2013@yandex.ru> Co-authored-by: Savio <72797635+Savio-Sou@users.noreply.github.com> Co-authored-by: StoneMac65 <StoneMac65@gmail.com> Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Zachary James Williamson <blorktronics@gmail.com> Co-authored-by: benesjan <13470840+benesjan@users.noreply.github.com> Co-authored-by: benesjan <janbenes1234@gmail.com> Co-authored-by: critesjosh <18372439+critesjosh@users.noreply.github.com> Co-authored-by: danielntmd <162406516+danielntmd@users.noreply.github.com> Co-authored-by: danielntmd <danielntmd@nethermind.io> Co-authored-by: dbanks12 <david@aztec-labs.com> Co-authored-by: defkit <84741533+defkit@users.noreply.github.com> Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com> Co-authored-by: fcarreiro <facundo@aztecprotocol.com> Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Co-authored-by: iAmMichaelConnor <mike@aztecprotocol.com> Co-authored-by: jeanmon <jean@aztec-labs.com> Co-authored-by: jfecher <jfecher11@gmail.com> Co-authored-by: josh crites <critesjosh@gmail.com> Co-authored-by: josh crites <jc@joshcrites.com> Co-authored-by: just-mitch <68168980+just-mitch@users.noreply.github.com> Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Co-authored-by: ledwards2225 <l.edwards.d@gmail.com> Co-authored-by: lucasxia01 <lucasxia01@gmail.com> Co-authored-by: ludamad <163993+ludamad@users.noreply.github.com> Co-authored-by: ludamad <adam.domurad@gmail.com> Co-authored-by: ludamad <domuradical@gmail.com> Co-authored-by: maramihali <mara@aztec-labs.com> Co-authored-by: maramihali <mara@aztecprotocol.com> Co-authored-by: mralj <nikola.mratinic@gmail.com> Co-authored-by: nventuro <2530770+nventuro@users.noreply.github.com> Co-authored-by: saleel <saleel@aztecprotocol.com> Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com> Co-authored-by: sirasistant <sirasistant@gmail.com> Co-authored-by: spypsy <spypsy@users.noreply.github.com> Co-authored-by: thunkar <gregojquiros@gmail.com> Co-authored-by: Álvaro Rodríguez <sirasistant@gmail.com>

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`

guipublic requested review from federicobarbacovi and ledwards2225 August 29, 2025 16:18

iakovenkos reviewed Aug 29, 2025

View reviewed changes

iakovenkos mentioned this pull request Sep 1, 2025

chore: bool_t from witness index functionality #16688

Merged

AztecBot force-pushed the gd/acir-serialization-changes branch from bfe9f90 to 7430692 Compare September 3, 2025 12:34

AztecBot requested review from TomAFrench, charlielye, dbanks12, fcarreiro and sirasistant as code owners September 3, 2025 12:34

AztecBot force-pushed the gd/acir-serialization-changes branch from 656eba3 to 44a5a55 Compare September 3, 2025 14:06

fcarreiro removed their request for review September 3, 2025 15:42

AztecBot force-pushed the gd/acir-serialization-changes branch from 8521ef0 to 88a944c Compare September 3, 2025 15:52

Base automatically changed from gd/acir-serialization-changes to next September 3, 2025 16:37

guipublic requested a review from LeilaWang as a code owner September 4, 2025 17:42

kashbrti added the ci-full Run all master checks. label Sep 25, 2025

ledwards2225 requested changes Sep 30, 2025

View reviewed changes

guipublic mentioned this pull request Oct 2, 2025

ACIR vs Brillig: ecdsa verify with predicate=0 failed to compile in Brillig and solved in ACIR noir-lang/noir#10037

Closed

federicobarbacovi reviewed Oct 3, 2025

View reviewed changes

guipublic mentioned this pull request Oct 6, 2025

[BUG] (Barretebberg) batch_mul in cycle_group does not handle 'mixed' constant-ness #17514

Closed

TomAFrench reviewed Oct 8, 2025

View reviewed changes

Comment thread barretenberg/acir_tests/bootstrap.sh Outdated

TomAFrench reviewed Oct 9, 2025

View reviewed changes

Comment thread barretenberg/acir_tests/bootstrap.sh Outdated

ledwards2225 self-requested a review October 9, 2025 14:15

ledwards2225 approved these changes Oct 9, 2025

View reviewed changes

guipublic enabled auto-merge October 13, 2025 12:41

TomAFrench reviewed Oct 13, 2025

View reviewed changes

Comment thread .test_patterns.yml Outdated

TomAFrench approved these changes Oct 13, 2025

View reviewed changes

TomAFrench added the ci-squash-and-merge label Oct 13, 2025

AztecBot removed the ci-squash-and-merge label Oct 13, 2025

AztecBot force-pushed the gd/bb_predicates branch from fcc0b73 to e2bf48e Compare October 13, 2025 16:12

TomAFrench force-pushed the gd/bb_predicates branch from e2bf48e to 2764b96 Compare October 13, 2025 16:37

guipublic added this pull request to the merge queue Oct 13, 2025

Merged via the queue into next with commit 4d916dd Oct 13, 2025
14 checks passed

guipublic deleted the gd/bb_predicates branch October 13, 2025 18:53

ludamad pushed a commit that referenced this pull request Dec 16, 2025

chore: use the bb predicates when generating constraints (#16663)

1427a0f

ludamad pushed a commit that referenced this pull request Dec 16, 2025

chore: use the bb predicates when generating constraints (#16663)

1dd3946

-    if (!input.predicate.is_constant) {
+    if (!input.predicate.is_constant) {
+        // We need to use a point which is different from the generator otherwise secp256r1 ECDSA verification fails
+        auto default_point = Curve::g1::one + Curve::g1::one;
+        bool_ct predicate_witness =
+            bool_ct::from_witness_index_unsafe(&builder, input.predicate.index);
+        pub_x = Fq::conditional_assign(predicate_witness, pub_x, default_point.x);
+        pub_y = Fq::conditional_assign(predicate_witness, pub_y, default_point.y);
+    } else {
+        BB_ASSERT_EQ(input.predicate.value, true, "Creating ECDSA constraints with a constant predicate equal to false.");
+    }

Conversation

guipublic commented Aug 29, 2025 • edited by kashbrti Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomAFrench commented Sep 4, 2025

Uh oh!

ledwards2225 commented Sep 5, 2025

Uh oh!

TomAFrench commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ledwards2225 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

federicobarbacovi Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

federicobarbacovi Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ledwards2225 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TomAFrench left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

guipublic commented Aug 29, 2025 •

edited by kashbrti

Loading

TomAFrench commented Sep 5, 2025 •

edited

Loading

federicobarbacovi Oct 3, 2025 •

edited

Loading

federicobarbacovi Oct 3, 2025 •

edited

Loading