feat: split public inputs from proof by lucasxia01 · Pull Request #12816 · AztecProtocol/aztec-packages

lucasxia01 · 2025-03-17T21:09:28Z

Splits the UH proof into the "inner" public inputs and the rest of the proof.

The "inner" public inputs are the public inputs that the user usually cares about as we will add a pairing point accumulator to the public inputs internally for recursion purposes. The rest of the proof will contain the pairing point accumulator and then all of the witness commitments, sumcheck univariates, etc...

…uts-from-proof

charlielye

I'm approving the bootstrap.sh changes (codeowner).
Might want someone else to review rest.

…rotocol/aztec-packages into lx/split-public-inputs-from-proof

lucasxia01 · 2025-03-28T02:48:18Z

yarn-project/stdlib/src/proofs/proof.ts


+  // This function assumes that the proof will contain an aggregation object
  public extractPublicInputs(): Fr[] {
+    if (this.isEmpty()) {


had to add extra if statements here. No idea how it was working before without error but I guess the old logic might've just somehow gotten an array of all 0s

please add asserts and comments around this code if possible

yarn-project/bb-prover/src/bb/execute.ts

lucasxia01 · 2025-03-28T02:49:22Z

yarn-project/bb-prover/src/prover/bb_prover.ts

-      const rawProof = await fs.readFile(`${provingResult.proofPath!}/${PROOF_FILENAME}`);
      const vkData = this.getVerificationKeyDataForCircuit(circuitType);
-      const proof = new Proof(rawProof, vkData.numPublicInputs);
+      const proof = await this.readProofAsFields(provingResult.proofPath!, vkData, RECURSIVE_PROOF_LENGTH);


changed this code to just call readProofAsFields so there's less redundancy

barretenberg/cpp/src/barretenberg/api/write_prover_output.hpp

lucasxia01 · 2025-03-28T02:51:39Z

yarn-project/bb-prover/src/prover/bb_prover.ts


-    const fieldsWithoutPublicInputs = json.slice(numPublicInputs).map(Fr.fromHexString);
+    // concat binary public inputs and binary proof
+    const binaryProofWithPublicInputs = Buffer.concat([binaryPublicInputs, binaryProof]);


both the proof and public inputs now contain the 4 bytes of metadata, so this buffer will look like:
[4 bytes of metadata for public inputs, binary public inputs, 4 bytes of metadata for proof, binary proof]

will you add the structure in a comment. also would it be possible to add asserts to make sure the metadata is placed where it's supposed to be?

Don't think so because its all just bytes. But I can add a check on the length instead

yarn-project/stdlib/src/proofs/proof.ts

maramihali

Thank you for doing this very painful work and guiding me through the PR. My comments are requests for TODOs, documenting and asserts but you are in the best position to judge where exactly these asserts should be. Especially I think the positioning of metadata and whether this somehow gets overwritten should be made explicit if possible.

maramihali · 2025-03-28T12:33:07Z

barretenberg/cpp/src/barretenberg/api/api_client_ivc.cpp

 }

 bool ClientIVCAPI::verify([[maybe_unused]] const Flags& flags,
+                          [[maybe_unused]] const std::filesystem::path& public_inputs_path,


will you add TODOs (+issues) in all places that have hacks to not lose track of them?

maramihali · 2025-03-28T12:37:33Z

barretenberg/acir_tests/sol-test/src/index.js

@@ -218,43 +218,51 @@ const killAnvil = () => {



please add an issue

maramihali · 2025-03-28T12:38:49Z

barretenberg/acir_tests/sol-test/src/index.js

-    publicInputsPath = getEnvVar("PUBLIC_INPUTS");
+    publicInputsAsFieldsPath = getEnvVar("PUBLIC_INPUTS_AS_FIELDS");
  } catch (e) {
    // noop


can we have some explicit error message in case we hit these catch-es?

It's not really an error in this case. It's expected for some of the flows

maramihali · 2025-03-28T12:41:38Z

barretenberg/acir_tests/sol-test/src/index.js

@@ -218,43 +218,51 @@ const killAnvil = () => {



also I agree, hopefully at some point we stop handling the plonk scenario - also I'm surprised this has anything to do with bbjs, I regarded this file as only useful to unit test that we can deploy the verifier contract (after we ensured that all forge tests pass so the contract is presumably correct)

maramihali · 2025-03-28T12:44:43Z

barretenberg/cpp/src/barretenberg/api/api_ultra_honk.cpp

 {
    auto prover = _compute_prover<Flavor>(bytecode_path.string(), witness_path.string(), init_kzg_accumulator);
-    return { prover.construct_proof(), compute_vk ? std::make_shared<VK>(prover.proving_key->proving_key) : nullptr };
+    HonkProof concat_pi_and_proof = prover.construct_proof();


can we add some asserts / comments to catch potential errors around this code?

yarn-project/bb-prover/src/bb/execute.ts

maramihali · 2025-03-28T12:50:42Z

yarn-project/bb-prover/src/prover/bb_prover.ts


-    const fieldsWithoutPublicInputs = json.slice(numPublicInputs).map(Fr.fromHexString);
+    // concat binary public inputs and binary proof
+    const binaryProofWithPublicInputs = Buffer.concat([binaryPublicInputs, binaryProof]);


will you add the structure in a comment. also would it be possible to add asserts to make sure the metadata is placed where it's supposed to be?

yarn-project/stdlib/src/proofs/proof.ts

maramihali · 2025-03-28T12:51:50Z

yarn-project/stdlib/src/proofs/proof.ts


+  // This function assumes that the proof will contain an aggregation object
  public extractPublicInputs(): Fr[] {
+    if (this.isEmpty()) {


please add asserts and comments around this code if possible

maramihali · 2025-03-28T12:52:34Z

yarn-project/stdlib/src/proofs/proof.ts

-      this.buffer.subarray(this.metadataOffset, this.metadataOffset + Fr.SIZE_IN_BYTES * this.numPublicInputs),
+      this.buffer.subarray(
+        this.metadataOffset,
+        this.metadataOffset + Fr.SIZE_IN_BYTES * (this.numPublicInputs - AGGREGATION_OBJECT_LENGTH),


please create constants for sizes that are used several times and asserts if possible

maramihali · 2025-03-28T13:03:25Z

whenever I approve from the VS code extension it does weird things such as posting comments twice - sorry 😅

…uts-from-proof

🤖 I have created a new Aztec Packages release --- ## [0.83.0](v0.82.3...v0.83.0) (2025-04-03) ### ⚠ BREAKING CHANGES * operation mouthwash ([#13171](#13171)) * change version and protocol version to rollupVersion ([#13145](#13145)) * processing events in Aztec.nr ([#12957](#12957)) ### Features * accept multiple consensus client endpoints ([#13022](#13022)) ([1d0185d](1d0185d)) * add ecdsa non ssh account to cli wallet ([#13085](#13085)) ([c5f9984](c5f9984)) * Add parent log link to top of CI logs. ([#13170](#13170)) ([a851087](a851087)) * Add public data read gadget ([#13138](#13138)) ([6bb76db](6bb76db)) * **avm:** merkle db hints (part 3) ([#13199](#13199)) ([7f96676](7f96676)) * **Barretenberg:** static analyzer's routine ([#13207](#13207)) ([84890c2](84890c2)) * derived pending notes capsules slot ([#13102](#13102)) ([6307ba0](6307ba0)) * get mana limit from rollup by default. ([#13029](#13029)) ([a406c54](a406c54)) * Increase CIVC depth with no rollup cost ([#13106](#13106)) ([3a555ef](3a555ef)) * making SyncDataProvider throw before sync ([#13151](#13151)) ([9840241](9840241)) * more benchmarks ([#13103](#13103)) ([7a2c9b7](7a2c9b7)) * **noir:** Allow missing optional fields in msgpack ([#13141](#13141)) ([493dede](493dede)) * processing events in Aztec.nr ([#12957](#12957)) ([88c0e04](88c0e04)) * Prover id defaults to publisher address ([#13206](#13206)) ([f459c3e](f459c3e)) * purge of log decryption in TS ([#12992](#12992)) ([800ab8d](800ab8d)) * register private-only contracts in cli-wallet and misc improvements ([#13245](#13245)) ([ad0530f](ad0530f)) * remove unary trick in decomposition and constraints polishing ([#13080](#13080)) ([0e60255](0e60255)) * split public inputs from proof ([#12816](#12816)) ([ca7151e](ca7151e)) * use magic address for fee juice portal in msgs ([#13241](#13241)) ([6fbc378](6fbc378)) * util for computing proposer/forwarder address ([#13169](#13169)) ([87809b2](87809b2)) * Zw/goblin avm tests ([#12904](#12904)) ([baea4b3](baea4b3)) ### Bug Fixes * add check for rollup version in tx validator ([#13197](#13197)) ([c8220c9](c8220c9)), closes [#13192](#13192) * add flake ([13bdfa7](13bdfa7)) * add flakes ([547d50a](547d50a)) * **avm:** alu interface ([#13115](#13115)) ([101ff78](101ff78)) * await transaction to make proposal ([#13132](#13132)) ([180db9e](180db9e)) * bb mac publish on tag push ([#13174](#13174)) ([8d46c8c](8d46c8c)) * bb workdir permission issue ([#13095](#13095)) ([3685a80](3685a80)) * **bb:** dont publish .zst ([#13173](#13173)) ([4dba4e9](4dba4e9)) * boolean config helper for cli args works now ([#13110](#13110)) ([a93ce6e](a93ce6e)) * **docs:** Register FPC docs, contract deployment, events ([#13222](#13222)) ([49aabfb](49aabfb)) * don't log sepolia acc mnemonic on github action ([#13178](#13178)) ([809b44d](809b44d)) * eq instead of !== ([#13161](#13161)) ([921e347](921e347)) * fetch the correct vk in getSolidityVerifier ([#13157](#13157)) ([71b6719](71b6719)) * force anvil/blob networking to ipv4 on localhost. attempt to fix port flakes ([#13099](#13099)) ([970dae5](970dae5)) * fuzzing build issues ([#13114](#13114)) ([bacae3d](bacae3d)) * Handle proven chain events referring to unseen blocks ([#13144](#13144)) ([229515f](229515f)), closes [#13142](#13142) * handling multiple identical logs in a tx ([#13184](#13184)) ([c100499](c100499)) * incorrect blocknumber in syncTaggedLogs ([e58ea95](e58ea95)) * indexeddb multimap dupes ([#13254](#13254)) ([1e470f0](1e470f0)) * load two more points ([#13119](#13119)) ([2a2904a](2a2904a)) * Masternet to run with the nightly tag ([#13239](#13239)) ([f16b70e](f16b70e)) * newline weirdness in package.json ([#13111](#13111)) ([244ea99](244ea99)) * nightly deploy quotes ([#13253](#13253)) ([775eecf](775eecf)) * prover config read from L1 ([#13237](#13237)) ([13426fe](13426fe)) * Race condition while unwinding blocks ([#13148](#13148)) ([1c2291a](1c2291a)) * read and pass rollup version ([#13232](#13232)) ([4ee1e4a](4ee1e4a)) * recursive sumcheck bugs ([#12885](#12885)) ([a784802](a784802)) * reenable test cache ([4dfca94](4dfca94)) * remove [skip ci] edge condition ([#13228](#13228)) ([4e699ea](4e699ea)) * separator in pending partial notes capsule array slot ([#13153](#13153)) ([0d6ec63](0d6ec63)) * sysdig oops ([5cf6ad1](5cf6ad1)) * tagging bug ([#13061](#13061)) ([280e1a6](280e1a6)) * test tracking failures in merge queue ([#13235](#13235)) ([650fdc1](650fdc1)) * title and external check need to run in merge queue ([e326ed6](e326ed6)) * Transpile cmov ([#13194](#13194)) ([6b94555](6b94555)) * trying to fix EADDRINUSE ([#13176](#13176)) ([260a057](260a057)) * Uninstall gossipsub event handler on service stop ([#13190](#13190)) ([081f30d](081f30d)) * use version from registry for rollup instead of config ([#12938](#12938)) ([7dac390](7dac390)) * validate private double spends in txs with public funcs ([#13088](#13088)) ([4555871](4555871)) * yolo flake and alert fix ([f8de52b](f8de52b)) ### Miscellaneous * add container id to netlog ([9ecdf15](9ecdf15)) * Add e2e test to bootstrap test_cmds ([#13146](#13146)) ([09e4722](09e4722)) * add rollup version as universal cli option ([#13205](#13205)) ([#13213](#13213)) ([568d9e9](568d9e9)) * Add ultra versions of fuzzers in stdlib ([#13139](#13139)) ([aea210b](aea210b)) * align TS and C++ indexed leaf types ([#13185](#13185)) ([d2574cc](d2574cc)) * Assign bb test flake ([#13127](#13127)) ([69fdb04](69fdb04)) * **avm:** remove check_interaction from tests ([#13136](#13136)) ([7d875a6](7d875a6)) * change version and protocol version to rollupVersion ([#13145](#13145)) ([24d7f8b](24d7f8b)) * Cleanup config vars for alpha-testnet ([#13204](#13204)) ([153a606](153a606)) * convenient way to run app ivc from bb ([#13158](#13158)) ([cb1a857](cb1a857)) * Cron snapshot upload in spartan ([#13108](#13108)) ([7c520a8](7c520a8)) * Do not close store before stopping p2p client in tests ([#13223](#13223)) ([63a843e](63a843e)) * docker flake diagnostics. ([48da272](48da272)) * **docs:** alpha-testnet versioning ([#13016](#13016)) ([9ca5d3c](9ca5d3c)) * **docs:** Update CLI faucet command ([#13104](#13104)) ([6eb71de](6eb71de)) * **docs:** Update versions-updating.md ([#13090](#13090)) ([0310d4e](0310d4e)) * Enable debug logging for annoying unit tests ([#13191](#13191)) ([5556524](5556524)) * enable sentinel in sentinel test ([#13219](#13219)) ([f2cfe3f](f2cfe3f)) * Exclude nightly versions from stable fn ([#13196](#13196)) ([a832e90](a832e90)) * Fix flake in e2e fees failures ([#13229](#13229)) ([5defe47](5defe47)) * fix kind logs ([#13023](#13023)) ([c6c2727](c6c2727)), closes [#13053](#13053) * fuzzing build in ci ([#13105](#13105)) ([1c08d38](1c08d38)) * Improve callstacks for public dispatch fns ([#13120](#13120)) ([f67375d](f67375d)) * log out the slash factory when a new rollup is deployed ([#13131](#13131)) ([4643a31](4643a31)) * Merge alpha back to master ([#13128](#13128)) ([504c338](504c338)) * metric attributes ([#13126](#13126)) ([f87d5e3](f87d5e3)), closes [#13063](#13063) * minor tagging API improvement ([#13092](#13092)) ([f5bcece](f5bcece)) * more benchmarking ([#13211](#13211)) ([4b0a4ad](4b0a4ad)) * move unbound impl generics to their functions ([#13147](#13147)) ([62f497f](62f497f)) * operation mouthwash ([#13171](#13171)) ([384f4e5](384f4e5)) * remove catch-all branch from opcode match in transpiler ([#13210](#13210)) ([2bc9ca2](2bc9ca2)) * remove templating by flavor in merge protocol ([#13098](#13098)) ([cf5e217](cf5e217)) * rename journal dir and file to state manager & mv to up to public/ ([#13159](#13159)) ([f13be09](f13be09)) * replace relative paths to noir-protocol-circuits ([f18336d](f18336d)) * replace relative paths to noir-protocol-circuits ([d620c12](d620c12)) * replace relative paths to noir-protocol-circuits ([be3e74e](be3e74e)) * replace relative paths to noir-protocol-circuits ([5bfac63](5bfac63)) * replace relative paths to noir-protocol-circuits ([f41ab2c](f41ab2c)) * replace relative paths to noir-protocol-circuits ([806e560](806e560)) * streamlined log processing ([#13107](#13107)) ([b184865](b184865)) * use testnet optimized trace ([#13135](#13135)) ([5a4f2ac](5a4f2ac)) * uses a new larger redis cache. flake file tweaks. ([#13167](#13167)) ([0951fb6](0951fb6)) ### Documentation * Add intermediate migration note versions ([#13183](#13183)) ([f5de7b8](f5de7b8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

lucasxia01 added 3 commits March 12, 2025 21:29

new types

adc4c74

update prover output

f79e79b

bb compiles

4eecccf

lucasxia01 self-assigned this Mar 17, 2025

lucasxia01 marked this pull request as ready for review March 17, 2025 21:34

lucasxia01 added 2 commits March 18, 2025 14:54

Merge remote-tracking branch 'origin/master' into lx/split-public-inp…

240d1a9

…uts-from-proof

update acir tests bootstrap

6f9666b

lucasxia01 requested a review from charlielye as a code owner March 18, 2025 15:50

lucasxia01 added 13 commits March 19, 2025 18:31

update UH verify to take in public inputs from a separate file

5327e57

fix acir tests for solidity flows

fe4e9f0

changed vk hashes

aba0481

Merge remote-tracking branch 'origin/master' into lx/split-public-inp…

b9edfe4

…uts-from-proof

update to make plonk sol acir tests work

1cc406b

Merge remote-tracking branch 'origin/master' into lx/split-public-inp…

bc0ffe9

…uts-from-proof

update public-inputs to public_inputs

f1b2945

update the prove and verify to use the public_inputs_path

92f1d50

try to fix things

df407ba

Merge remote-tracking branch 'origin/master' into lx/split-public-inp…

25e097b

…uts-from-proof

fix writing json for empty arrays

4edea76

fix sol acir tests

0556524

fix public inputs path

5ab1dea

charlielye approved these changes Mar 24, 2025

View reviewed changes

lucasxia01 added 8 commits March 24, 2025 15:44

update bbjs_prove_bb_verify to split public inputs

3448f64

attempt to fix bbjs sol flow

1bf4645

oops, not the correct fix

36a629f

fix prove_tube

08c1dac

Merge branch 'master' into lx/split-public-inputs-from-proof

3e04edf

Merge branch 'master' into lx/split-public-inputs-from-proof

f78066c

fix the gd full test

69de125

Merge branch 'lx/split-public-inputs-from-proof' of github.com:AztecP…

90438e4

…rotocol/aztec-packages into lx/split-public-inputs-from-proof

lucasxia01 added 5 commits March 26, 2025 22:47

fix execute.ts format

fb775fe

fix prover node publisher test

31f0d17

Merge branch 'master' into lx/split-public-inputs-from-proof

0245191

move the hack downward because typescript.

52dd528

Merge branch 'lx/split-public-inputs-from-proof' of github.com:AztecP…

175915b

…rotocol/aztec-packages into lx/split-public-inputs-from-proof

saleel mentioned this pull request Mar 27, 2025

feat: Make public inputs the start of the UH and MH proof #12266

Merged

fix prove tube test

fc47c55

lucasxia01 commented Mar 28, 2025

View reviewed changes

yarn-project/bb-prover/src/bb/execute.ts Show resolved Hide resolved

lucasxia01 commented Mar 28, 2025

View reviewed changes

barretenberg/cpp/src/barretenberg/api/write_prover_output.hpp Show resolved Hide resolved

lucasxia01 commented Mar 28, 2025

View reviewed changes

yarn-project/stdlib/src/proofs/proof.ts Outdated Show resolved Hide resolved

lucasxia01 requested a review from maramihali March 28, 2025 03:00

maramihali approved these changes Mar 28, 2025

View reviewed changes

lucasxia01 added 4 commits March 28, 2025 14:54

add comments and todos

09f2800

Merge remote-tracking branch 'origin/master' into lx/split-public-inp…

635971c

…uts-from-proof

add TODO

68d1081

changing import to fix build

786889f

lucasxia01 mentioned this pull request Mar 31, 2025

Proper handling of the pairing point object in yarn-project #13188

Open

fix bb prover full rollup test

15cdc65

lucasxia01 mentioned this pull request Mar 31, 2025

Umbrella Issue for barretenberg and yarn-project intersection #13189

Open

fix prover-node-publisher test

ea6e789

lucasxia01 enabled auto-merge (squash) March 31, 2025 22:02

lucasxia01 merged commit ca7151e into master Mar 31, 2025
7 checks passed

lucasxia01 deleted the lx/split-public-inputs-from-proof branch March 31, 2025 22:11

AztecBot mentioned this pull request Mar 31, 2025

chore(master): release 0.83.0 #13112

Merged

lucasxia01 mentioned this pull request Apr 4, 2025

Separate the public inputs AztecProtocol/barretenberg#1264

Closed

Conversation

lucasxia01 commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

charlielye left a comment

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!

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

maramihali left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maramihali commented Mar 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lucasxia01 commented Mar 17, 2025 •

edited

Loading