chore: no PK by ledwards2225 · Pull Request #15386 · AztecProtocol/aztec-packages

ledwards2225 · 2025-06-29T03:43:04Z

Removes the ProvingKey altogether from the Ultra/Mega proving systems.

TLDR: "proving key" used to mean "the precomputable data used by the prover." The Honk PK hasn't been that for a long time since it stores the ProverPolynomials which contains all polynomials used by the prover, including witnesses. This meant there was no clear logic behind what was stored in the DeciderProvingKey vs the ProvingKey. Since we don't have much use for the traditional notion of a proving key in Honk (unlike in Plonk), I've opted to remove it entirely and place its data directly in the DeciderProvingKey.

Most lines in this PR relate to one of these changes:

There is a single circuit-size-like parameter in the DPK, accessed via dyadic_size()
A honk VK is now constructed as vk(pk->get_precomputed()) (instead of vk(pk->proving_key))
Lines containing proving_key->proving_key.foo have been reduced to proving_key->foo
"metadata" (circuit size and public inputs metadata) live in a MetaData struct. (Note: I'm open to a better name..)

…tered (#15313) We make the folding of Apps share a transcript until a kernel is encountered.

Bigfield internal audit related cleanup/fixes. - Simplified data structs: `non_native_multiplication_witnesses` and `non_native_partial_multiplication_witnesses` - Reduced code duplication with new functions like `get_binary_basis_limb_witness_indices` - Resolved/removed some of the TODOs resolves #14662 #14660 resolves AztecProtocol/barretenberg#999 resolves #14658

Closes AztecProtocol/barretenberg#1114 Closes AztecProtocol/barretenberg#1144 Closes AztecProtocol/barretenberg#1283 Closes AztecProtocol/barretenberg#1159

ledwards2225 · 2025-06-29T17:47:38Z

-    }
-}
-
-template <class Flavor>


This didn't belong here - moved to the DPK

fcarreiro · 2025-06-30T11:14:59Z

Should we consider a bigger change in the AVM to remove ProvingKey or at least rename it? (our team, I mean).

My understanding is that ProverPolynomials are equivalent to ProvingKey without the shifted entities in the AVM. Therefore, I am wondering if we really need ProvingKey to be defined.
I would say a rename might not really be worth it. If we can easily remove it then let us do it (could be a task once AVM is fully constrained though.)

It would likely be quite a simple change to get rid of it but the AVM usage of the PK has already diverged from the rest of the proving systems so I didn't want to mess around there

Ok! created #15402

lucasxia01 · 2025-07-01T14:24:31Z

    std::shared_ptr<ClientIVC::DeciderProvingKey> proving_key = get_acir_program_decider_proving_key(program);
-    auto verification_key = std::make_shared<ClientIVC::MegaVerificationKey>(proving_key->proving_key);
+    auto verification_key =
+        std::make_shared<ClientIVC::MegaVerificationKey>(proving_key->polynomials, proving_key->metadata);


maybe we should rename all of these to be decider_proving_key?

although maybe thats a bad name too

would like to rename but I'll leave that for a follow on. I think maybe instance should come back

lucasxia01 · 2025-07-03T17:23:33Z

@@ -169,9 +143,9 @@ template <typename FF, typename CommitmentKey_> class ProvingKey_ {
 template <typename PrecomputedCommitments> class NativeVerificationKey_ : public PrecomputedCommitments {


why not use Metadata in this class?

I briefly went down that path but its a bit messy because of all the different pub input keys. Once those go away I'll reconsider

lucasxia01

Looks good in general, just left a bunch of comments

lucasxia01 · 2025-07-03T17:26:21Z

-                commitment = ck.commit(polynomial);
+            set_metadata(metadata);
+
+            CommitmentKey commitment_key{ metadata.circuit_size };


now, we always create a new commitment key, but maybe we should maintain the previous logic of only creating a new one if necessary, although that logic seems icky.

we now just have a persistent srs so constructing a new one is cheap and free as long as the one in the PK is still around so this is basically the same logic that existed before

what happens if the new one requires more points? I guess it just allocates the extra points?

lucasxia01 · 2025-07-03T17:30:51Z

+            this->num_public_inputs = metadata.num_public_inputs;
+            this->pub_inputs_offset = metadata.pub_inputs_offset;
+            this->pairing_inputs_public_input_key = metadata.pairing_inputs_public_input_key;
+            this->ipa_claim_public_input_key = metadata.ipa_claim_public_input_key;


why will they go away?

lucasxia01 · 2025-07-03T18:21:05Z

+            this->num_public_inputs = metadata.num_public_inputs;
+            this->pub_inputs_offset = metadata.pub_inputs_offset;
+            this->pairing_inputs_public_input_key = metadata.pairing_inputs_public_input_key;
+            this->ipa_claim_public_input_key = metadata.ipa_claim_public_input_key;


actually, I'm also confused why they weren't there before

lucasxia01 · 2025-07-07T14:34:51Z

        VerificationKey(const VerificationKey& vk) = default;

-        void set_metadata(const ProvingKey& proving_key)
+        void set_metadata(const MetaData& metadata)


interesting that this doesn't exist for other flavors

lucasxia01 · 2025-07-07T16:53:11Z

+    if (incoming->get_overflow_size() > accumulator->get_overflow_size()) {
+        std::swap(accumulator->polynomials, incoming->polynomials); // swap the polys
+        std::swap(lagranges[0], lagranges[1]);                 // swap the lagrange coefficients so the sum is unchanged
+        accumulator->set_dyadic_size(incoming->dyadic_size()); // update dyadic size of accumulator


there's no swapping here anymore?

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>

AztecBot and others added 22 commits June 27, 2025 02:02

[empty] Start merge-train. Choo choo.

e3d7312

chore: update merge-train documentation

d2415e7

Merge branch 'next' into merge-train/barretenberg [skip ci]

1487ee7

feat!: (sequence of) Apps share a transcript until a kernel is encoun…

3bdb488

…tered (#15313) We make the folding of Apps share a transcript until a kernel is encountered.

Merge branch 'next' into merge-train/barretenberg [skip ci]

e254a39

Merge branch 'next' into merge-train/barretenberg [skip ci]

c8305d1

Merge branch 'next' into merge-train/barretenberg [skip ci]

f4d89f7

Merge branch 'next' into merge-train/barretenberg [skip ci]

f23d9a5

Merge branch 'next' into merge-train/barretenberg [skip ci]

232e2a4

Merge branch 'next' into merge-train/barretenberg [skip ci]

7507e7b

chore: remove insecure logic from ipa recursion (#14102)

c10a713

Closes AztecProtocol/barretenberg#1114 Closes AztecProtocol/barretenberg#1144 Closes AztecProtocol/barretenberg#1283 Closes AztecProtocol/barretenberg#1159

Merge branch 'next' into merge-train/barretenberg [skip ci]

ff42054

lots of stuff moved to dpk, all is building

2022b36

no more use of pk directly in the dpk

a665f7c

introduce metadata, ick

8115a12

add new VK constructors from polys n metadata

ad64c0d

new vk constructor in uh tests

8eb1132

new vk constructor builds in all

ef39ad3

nom more polynomials in pks

43edfd9

no more PK in DPK

ff61484

PKs deleted altogether

efcdcfa

ledwards2225 changed the base branch from next to merge-train/barretenberg June 29, 2025 03:43

ledwards2225 added 5 commits June 29, 2025 04:10

store a single dyadic size param in the dpk

51b9a84

delete commented out code

312fd23

method for memory record population

11551f5

getter for num pub inputs

f096fe7

pk removed from base flavor

665bdca

ledwards2225 commented Jun 29, 2025

View reviewed changes

cleanup and reinstate some tests

c2abb77

ledwards2225 marked this pull request as ready for review June 29, 2025 20:57

ledwards2225 requested review from IlyasRidhuan, fcarreiro and jeanmon as code owners June 29, 2025 20:57

fcarreiro reviewed Jun 30, 2025

View reviewed changes

fcarreiro mentioned this pull request Jun 30, 2025

avm: get rid of proving key #15402

Open

lucasxia01 self-requested a review June 30, 2025 23:49

lucasxia01 reviewed Jul 1, 2025

View reviewed changes

AztecBot force-pushed the merge-train/barretenberg branch from 6e073a5 to 1829e9a Compare July 2, 2025 18:42

AztecBot requested review from LeilaWang and charlielye as code owners July 2, 2025 18:42

AztecBot force-pushed the merge-train/barretenberg branch 2 times, most recently from 141bfcd to 74bac59 Compare July 3, 2025 03:06

lucasxia01 reviewed Jul 3, 2025

View reviewed changes

Comment thread barretenberg/cpp/src/barretenberg/benchmark/mega_memory_bench/mega_memory.bench.cpp

lucasxia01 reviewed Jul 3, 2025

View reviewed changes

Comment thread barretenberg/cpp/src/barretenberg/flavor/flavor.hpp Outdated

lucasxia01 reviewed Jul 3, 2025

View reviewed changes

lucasxia01 approved these changes Jul 3, 2025

View reviewed changes

AztecBot force-pushed the merge-train/barretenberg branch from 913ed6b to 4bc2037 Compare July 4, 2025 02:11

ledwards2225 added 6 commits July 4, 2025 22:02

Merge branch 'merge-train/barretenberg' into lde/no_pk

aab76ec

rename circuit size to dyadic size

61740c3

review fixes

73b8d70

introduce PrecomputedData for VK constructor

a339d89

make metadat and some other stuff private

a524fb8

comment

efed809

ledwards2225 merged commit 6bf35ca into merge-train/barretenberg Jul 5, 2025
5 checks passed

ledwards2225 deleted the lde/no_pk branch July 5, 2025 23:41

lucasxia01 reviewed Jul 7, 2025

View reviewed changes

AztecBot mentioned this pull request Jul 8, 2025

feat: merge-train/barretenberg #15586

Merged

		@@ -169,9 +143,9 @@ template <typename FF, typename CommitmentKey_> class ProvingKey_ {
		template <typename PrecomputedCommitments> class NativeVerificationKey_ : public PrecomputedCommitments {

Conversation

ledwards2225 commented Jun 29, 2025 • edited 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!

fcarreiro Jun 30, 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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucasxia01 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!

lucasxia01 Jul 7, 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ledwards2225 commented Jun 29, 2025 •

edited

Loading

fcarreiro Jun 30, 2025 •

edited

Loading

lucasxia01 Jul 7, 2025 •

edited

Loading