feat: proof surgery class #8236

ledwards2225 · 2024-08-27T23:57:08Z

Adds a ProofSurgeon class that manages all proof surgery, e.g. splitting public inputs out of proof for acir and reconstructing again for bberg. Simplifies things quite a bit in the process.

ledwards2225 · 2024-08-28T17:53:28Z

barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp

+        // the proof and public_inputs we subtract and add the corresponding amount from the respective sizes.
+        size_t size_of_proof_with_no_pub_inputs = input.proof.size() - bb::AGGREGATION_OBJECT_SIZE;
+        size_t total_num_public_inputs = input.public_inputs.size() + bb::AGGREGATION_OBJECT_SIZE;
+        create_dummy_vkey_and_proof(


@lucas you were right that the moving of the agg object from proof to pub inputs was needed in a sense but it was just because this method expects the "raw" sizes of each. Rather than actually move it back and forth I just provide this method the adjusted sizes. BTW I think the long term solution for this dummy proof is just to read in some data from a file or something. This is too brittle

well we could just pass the unadjusted values to the create_dummy_vkey_and_proof function as well and handle it there

I actually did it that way first but found this to be slightly clearer since its external to the method where we know about the structure of the proof/pub inputs. Either way its kind of bad but its better than it was. I'll hopefully have time to clean all of this up more soon

ledwards2225 · 2024-08-28T17:55:23Z

barretenberg/cpp/src/barretenberg/dsl/acir_format/proof_surgeon.hpp

+     * @return RecursionWitnessData
+     */
+    static RecursionWitnessData populate_recursion_witness_data(bb::SlabVector<bb::fr>& witness,
+                                                                std::vector<bb::fr>& proof_witnesses,


This is a pretty substantial simplification of the old logic, driven in part by the fact that there's no reason (as far as I can tell) to place the data into witness in any special way, as long as the component indices are correct. So I just basically concatenate the key, proof and pub inputs into with witness without worrying about reinserting the pub inputs in the proof etc. This is also probably closer to how the witness looks coming from an actual noir program.

ledwards2225 · 2024-08-28T18:02:59Z

barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp

    // Fourth key field is the whether the proof contains an aggregation object.
-    builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index);
+    builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index); // WORKTODO: 4 = bug?


Also note this weird index that looks like a bug to me. If not then its definitely worth a comment explaining why

hm yeah, it should be 3...

not sure why we wouldn't get some sort of failure if we assert_equal the 4th index with both 1 and 0 variables...?

Yeah, a little concerning. My vague guess was that the 4th-index value was always non-zero and was somehow just getting converted to "true" somewhere along the way. not sure

lucasxia01

looks good in general

lucasxia01 · 2024-08-28T19:13:47Z

barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp

    // Fourth key field is the whether the proof contains an aggregation object.
-    builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index);
+    builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index); // WORKTODO: 4 = bug?


hm yeah, it should be 3...

lucasxia01 · 2024-08-28T19:14:32Z

barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp

    // Fourth key field is the whether the proof contains an aggregation object.
-    builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index);
+    builder.assert_equal(builder.add_variable(1), key_fields[4].witness_index); // WORKTODO: 4 = bug?


not sure why we wouldn't get some sort of failure if we assert_equal the 4th index with both 1 and 0 variables...?

lucasxia01 · 2024-08-28T19:16:14Z

barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp

+        // the proof and public_inputs we subtract and add the corresponding amount from the respective sizes.
+        size_t size_of_proof_with_no_pub_inputs = input.proof.size() - bb::AGGREGATION_OBJECT_SIZE;
+        size_t total_num_public_inputs = input.public_inputs.size() + bb::AGGREGATION_OBJECT_SIZE;
+        create_dummy_vkey_and_proof(


well we could just pass the unadjusted values to the create_dummy_vkey_and_proof function as well and handle it there

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.51.1</summary> ## [0.51.1](aztec-package-v0.51.0...aztec-package-v0.51.1) (2024-08-29) ### Features * Add status check to prover agent ([#8248](#8248)) ([7b3006a](7b3006a)) * Faster L1 deployment ([#8234](#8234)) ([51d6699](51d6699)) * Spartan token transfer ([#8163](#8163)) ([38f0157](38f0157)) </details> <details><summary>barretenberg.js: 0.51.1</summary> ## [0.51.1](barretenberg.js-v0.51.0...barretenberg.js-v0.51.1) (2024-08-29) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.51.1</summary> ## [0.51.1](aztec-packages-v0.51.0...aztec-packages-v0.51.1) (2024-08-29) ### Features * Add CLI command for gathering proving metrics ([#8221](#8221)) ([5929a42](5929a42)) * Add status check to prover agent ([#8248](#8248)) ([7b3006a](7b3006a)) * **avm:** 1-slot sload/sstore (nr, ts) ([#8264](#8264)) ([bdd9b06](bdd9b06)) * **avm:** Range check gadget ([#7967](#7967)) ([0dd954e](0dd954e)) * **docs:** Add partial notes doc ([#8192](#8192)) ([4299bbd](4299bbd)) * Faster L1 deployment ([#8234](#8234)) ([51d6699](51d6699)) * Initial validator set ([#8133](#8133)) ([6d31ad2](6d31ad2)) * L1-publisher cleanup ([#8148](#8148)) ([6ae2535](6ae2535)) * Proof surgery class ([#8236](#8236)) ([10d7edd](10d7edd)) * Request specific transactions through the p2p layer ([#8185](#8185)) ([54e1cc7](54e1cc7)) * Slot duration flexibility ([#8122](#8122)) ([708e4e5](708e4e5)) * Spartan token transfer ([#8163](#8163)) ([38f0157](38f0157)) ### Bug Fixes * Attempt to fix nightly test ([#8222](#8222)) ([477eec5](477eec5)) * **avm-simulator:** Await avm bytecode check ([#8268](#8268)) ([4410eb3](4410eb3)) * **bb-prover:** Create structure for AVM vk ([#8233](#8233)) ([55b6ba2](55b6ba2)) * **bb:** Mac build ([#8255](#8255)) ([ac54f5c](ac54f5c)) * **ci:** Spot-runner-action was not built ([#8274](#8274)) ([c1509c1](c1509c1)) * **ci:** Try fix brotli edge-case ([#8256](#8256)) ([e03ea0b](e03ea0b)) * Docker containers healthchecks ([#8228](#8228)) ([19edbbb](19edbbb)) * **docs:** Update entrypoint details on accounts page ([#8184](#8184)) ([8453ec7](8453ec7)) * Export brillig names in contract functions ([#8212](#8212)) ([4745741](4745741)) * Fixes for the nightly test run against Sepolia ([#8229](#8229)) ([cfc65c6](cfc65c6)) * Handle constant output for sha256 ([#8251](#8251)) ([0653ba5](0653ba5)) * Log public vm errors as warn in prover-agent ([#8247](#8247)) ([9f4ea9f](9f4ea9f)) * Remove devnet ARM builds for now ([#8202](#8202)) ([81ef715](81ef715)) * Remove fundFpc step from bootstrap ([#8245](#8245)) ([a742531](a742531)) * Ts codegen ([#8267](#8267)) ([cb58800](cb58800)) ### Miscellaneous * Add check to just release images to devnet-deploys ([#8242](#8242)) ([aa6791d](aa6791d)) * Add partial note support for value note ([#8141](#8141)) ([daa57cc](daa57cc)) * Always run `build-check` step in `publish-bb.yml` ([#8240](#8240)) ([5e9749f](5e9749f)) * **avm:** Replace range and cmp with gadgets ([#8164](#8164)) ([cc12558](cc12558)) * Basic network matrix ([#8257](#8257)) ([2a76b1a](2a76b1a)), closes [#8001](#8001) * **bb:** Use std::span in pippenger for scalars ([#8269](#8269)) ([2323cd5](2323cd5)) * Configure interval mining for anvil ([#8211](#8211)) ([eba57b4](eba57b4)) * Create external-ci-approved.yml ([#8235](#8235)) ([24b059b](24b059b)) * Disallow prune in devnet + add onlyOwners ([#8134](#8134)) ([c736f96](c736f96)) * Fix various warnings in noir code ([#8258](#8258)) ([1c6b478](1c6b478)) * Less noisy AVM failures in proving ([#8227](#8227)) ([03bcd62](03bcd62)) * Open an issue if publishing bb fails ([#8223](#8223)) ([2d7a775](2d7a775)) * Reinstate l1-contracts package ([#8250](#8250)) ([263a912](263a912)) * Remove unused generic parameters ([#8249](#8249)) ([00ed045](00ed045)) * Replace relative paths to noir-protocol-circuits ([1783c80](1783c80)) * Replace relative paths to noir-protocol-circuits ([ffe1f35](ffe1f35)) * Report prover metrics ([#8155](#8155)) ([dc7bcdf](dc7bcdf)), closes [#7675](#7675) * Rework balances map ([#8127](#8127)) ([1cac3dd](1cac3dd)), closes [#8104](#8104) * Run CI after merges to provernet ([#8244](#8244)) ([97e5e25](97e5e25)) ### Documentation * Minor fixes ([#8273](#8273)) ([2b8af9e](2b8af9e)) </details> <details><summary>barretenberg: 0.51.1</summary> ## [0.51.1](barretenberg-v0.51.0...barretenberg-v0.51.1) (2024-08-29) ### Features * **avm:** 1-slot sload/sstore (nr, ts) ([#8264](#8264)) ([bdd9b06](bdd9b06)) * **avm:** Range check gadget ([#7967](#7967)) ([0dd954e](0dd954e)) * Proof surgery class ([#8236](#8236)) ([10d7edd](10d7edd)) ### Bug Fixes * **bb-prover:** Create structure for AVM vk ([#8233](#8233)) ([55b6ba2](55b6ba2)) * **bb:** Mac build ([#8255](#8255)) ([ac54f5c](ac54f5c)) * Handle constant output for sha256 ([#8251](#8251)) ([0653ba5](0653ba5)) ### Miscellaneous * **avm:** Replace range and cmp with gadgets ([#8164](#8164)) ([cc12558](cc12558)) * **bb:** Use std::span in pippenger for scalars ([#8269](#8269)) ([2323cd5](2323cd5)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

ledwards2225 added 2 commits August 27, 2024 23:17

initial infrstructure of surgeon; tests passing

b9023a9

massive simplification to witness construction in test

5599967

ledwards2225 self-assigned this Aug 27, 2024

ledwards2225 added 5 commits August 28, 2024 14:48

no need to move agg obj to public inputs before reconstructing proof

8f43637

simplify witness pop method

3df7fc0

cleanup

7dd94c2

adjust sizes in dummy construction

6204cef

clarify comments around sizes

3595775

ledwards2225 commented Aug 28, 2024

View reviewed changes

ledwards2225 requested a review from lucasxia01 August 28, 2024 17:56

ledwards2225 marked this pull request as ready for review August 28, 2024 17:56

ledwards2225 commented Aug 28, 2024

View reviewed changes

lucasxia01 approved these changes Aug 28, 2024

View reviewed changes

fix bad index in dummy proof

302cdc1

ledwards2225 merged commit 10d7edd into master Aug 28, 2024
37 checks passed

ledwards2225 deleted the lde/proof_surgeon branch August 28, 2024 21:13

AztecBot mentioned this pull request Aug 28, 2024

chore(master): Release 0.51.1 #8218

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: proof surgery class #8236

feat: proof surgery class #8236

ledwards2225 commented Aug 27, 2024 •

edited

Loading

ledwards2225 Aug 28, 2024

lucasxia01 Aug 28, 2024

ledwards2225 Aug 28, 2024

ledwards2225 Aug 28, 2024 •

edited

Loading

ledwards2225 Aug 28, 2024

lucasxia01 Aug 28, 2024

lucasxia01 Aug 28, 2024

ledwards2225 Aug 28, 2024

lucasxia01 left a comment

lucasxia01 Aug 28, 2024

lucasxia01 Aug 28, 2024

lucasxia01 Aug 28, 2024

feat: proof surgery class #8236

feat: proof surgery class #8236

Conversation

ledwards2225 commented Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ledwards2225 Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasxia01 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ledwards2225 commented Aug 27, 2024 •

edited

Loading

ledwards2225 Aug 28, 2024 •

edited

Loading