[Perf] A revival of 1955 & 2145 #3127

ljedrz · 2026-01-28T15:32:39Z

This PR is a fresh implementation of #1955 combined with #2145, which built on it. Both were contained in a single commit each, and a few adjustments were necessary in order to work with the current codebase, but nothing too complex.

The affected benchmarks are:

Transaction::Execute(transfer_public) - verify
                        time:   [41.792 µs 41.808 µs 41.823 µs]
                        change: [−0.8837% −0.8428% −0.8003%] (p = 0.00 < 0.05)
                        Change within noise threshold.
// note: there were some outliers, but it would likely have been marked as an improvement without them

Transaction::Execute(transfer_private) - verify
                        time:   [46.640 µs 46.680 µs 46.718 µs]
                        change: [−2.3414% −2.2890% −2.2178%] (p = 0.00 < 0.05)
                        Performance has improved.

snark_certificate_prove_100000
                        time:   [109.25 ms 109.72 ms 110.19 ms]
                        change: [−3.3583% −2.7577% −2.1394%] (p = 0.00 < 0.05)
                        Performance has improved.

snark_certificate_verify_100000
                        time:   [93.865 ms 94.126 ms 94.391 ms]
                        change: [−2.3859% −1.9702% −1.5179%] (p = 0.00 < 0.05)
                        Performance has improved.

Filing as a draft, as backward compatibility (at least some serialization) still needs to be resolved.

CC @niklaslong

Signed-off-by: ljedrz <[email protected]>

… execution Signed-off-by: ljedrz <[email protected]>

Signed-off-by: ljedrz <[email protected]>

…patible Signed-off-by: ljedrz <[email protected]>

Signed-off-by: ljedrz <[email protected]>

iamalwaysuncomfortable

@vicsn made several comments.

Probably the most important is that for wasm we'll also need to build some tooling to adjust the parameters ahead of time. This requires some thought how to do correctly.

I'd like to proceed with this, would you suggest that Konstantin and I take on the role of carrying this to the finish line or do you recommend someone in protocol do it. Either way, I think we can get this in.

iamalwaysuncomfortable · 2026-01-30T01:00:28Z

algorithms/src/polycommit/sonic_pc/data_structures.rs


    /// The key used to commit to polynomials in Lagrange basis.
-    pub lagrange_bases_at_beta_g: BTreeMap<usize, Vec<E::G1Affine>>,
+    /// This is `None` if `self` does not support lagrange bases


Suggested change

/// This is `None` if `self` does not support lagrange bases

/// This is `None` if `self` does not support lagrange bases.

iamalwaysuncomfortable · 2026-01-30T01:09:59Z

algorithms/src/polycommit/sonic_pc/data_structures.rs

+    /// to the new degree_bounds The `powers_of_beta_times_gamma_g` is only
+    /// dependent on the `hiding_bound` so doesn't change This only works if
+    /// the SRS max_degree is fixed across specializations Note that this
+    /// implementation is not atomic. If specialize fails halfway through,


This comment should be completed.

iamalwaysuncomfortable · 2026-01-30T01:11:16Z

algorithms/src/polycommit/sonic_pc/data_structures.rs

+    pub fn update(&mut self, srs: &UniversalParams<E>, degree_info: &DegreeInfo) -> Result<()> {
+        let trim_time = start_timer!(|| "Trimming public parameters");
+
+        // The maximum degree required by the circuit polynomials


@copilot please add a commit that adds periods to all doc comments.

iamalwaysuncomfortable · 2026-01-30T01:12:54Z

algorithms/src/polycommit/kzg10/degree_info.rs

+
+use std::collections::BTreeSet;
+
+#[derive(Clone, Debug, Default)]


Suggested change

#[derive(Clone, Debug, Default)]

/// Degree info specifies information needed to determine which powers to use to build the CommitterKey from the SRS.

#[derive(Clone, Debug, Default)]

iamalwaysuncomfortable · 2026-01-30T03:08:47Z

algorithms/src/polycommit/sonic_pc/data_structures.rs

+                        let degree_diff = highest_degree_bound - new_highest_degree_bound;
+                        shifted_powers_of_beta_g = shifted_powers_of_beta_g.drain(degree_diff - 1..).collect_vec();


We may want to disable this in wasm and/or making trimming a manual call as we'll want to keep the SRS in place up to the max degree of the function in storage.

iamalwaysuncomfortable · 2026-01-30T03:24:38Z

algorithms/src/snark/varuna/ahp/indexer/circuit_info.rs

+            2 * self.num_constraints,                  // zerocheck poly degree
+            2 * self.num_public_and_private_variables, // lineval sumcheck poly degree
+            2 * self.num_non_zero_a,                   // matrix sumcheck poly degree
+            2 * self.num_non_zero_b,                   // matrix sumcheck poly degree
+            2 * self.num_non_zero_c,                   // matrix sumcheck poly degree


@vicsn this I think was possibly done before the changes in Arithmetization? Are these still valid?

Yeah I should double check this, thx for the ping

iamalwaysuncomfortable · 2026-01-30T03:48:56Z

algorithms/src/polycommit/sonic_pc/data_structures.rs

+                    Ordering::Greater => {
+                        let lowest_shift_degree = max_degree - new_highest_degree_bound;
+                        let highest_shift_degree = max_degree + 1 - highest_degree_bound;
+                        let mut new_powers = srs.powers_of_beta_g(lowest_shift_degree, highest_shift_degree)?;


Although this PR helps a ton by reducing key sizes, this becomes the new pain in the butt for those without filesystems because at the very best, it invokes synchronous XmlHttpRequest and that works (browsers are ending support for it) but it may block the app during the i/o. At worst, this fails and there's no access to the parameters.

There are three options to fix this:

Cache the pieces of the SRS fetched into memory somewhere and modify this logic slightly to fetch from cache.

Provide an extern function is implementable by outside callers which allows custom fetching logic, but validates param integrity with a checksum.

Find some way to do this asynchronously at an earlier point. The SDK is a custom executor, so it could potentially call parameter updates from degree bounds stated within pk/vks so that the SRS is properly trimmed by this point.

We'll have to do some current testing to ensure that XmlHttpRequest is still working in most browsers, otherwise we lose the ability to do local Proving if we DO have proving keys.

iamalwaysuncomfortable · 2026-01-30T03:53:39Z

algorithms/src/snark/varuna/ahp/prover/round_functions/third.rs

+        for (((circuit_specific_state, batch_combiner), assignments_i), matrix_transposes_i) in state
            .circuit_specific_states
-            .iter_mut()
+            .values_mut()


@ljedrz is this safe?

what aspect of safety do you have in mind? mutating values of a BTreeMap is perfectly fine, only the keys may not be modified

iamalwaysuncomfortable · 2026-01-30T04:02:06Z

algorithms/src/snark/varuna/data_structures/circuit_proving_key.rs

        let circuit_verifying_key = CanonicalDeserialize::deserialize_compressed(&mut reader)?;
        let circuit = CanonicalDeserialize::deserialize_compressed(&mut reader)?;
-        let committer_key = Arc::new(FromBytes::read_le(&mut reader)?);
+        let committer_key = match bool::read_le(&mut reader) {


This serialization isn't strictly backwards compatible, this will break for old proving keys because there's no flag supporting it in the original version. A serialization helper needs to be developed to handle legacy proving keys and thus this needs a bit more work to be fully backwards compatible.

@vicsn @ljedrz what's the bandwidth in protocol for this? Should start working on addressing our own comments?

Thx for the ping. Lukasz can make it backwards compatible.

I actually already had it, just needed to push; it's in 8083480.

iamalwaysuncomfortable · 2026-01-30T04:05:18Z

algorithms/src/snark/varuna/varuna.rs

+        let degree_info = degree_info.unwrap();

-            let ck = CommitterUnionKey::union(std::iter::once(&committer_key));
+        universal_prover.update(srs, degree_info)?;


This is a piece in wasm that we may have to call externally, which will necessarily mean we need the vks easily to crossreference all this info.

Could degree info potentially be gleaned from an authorization similarly to how the execution cost was? If so that'd give us a natural stopping point in the case where a wallet doesn't have all proving keys loaded (highly possible).

The degree info / CircuitInfo is in the synthesized ProvingKey/VerifyingKey. I think there's no way to quickly get it otherwise.

Okay that implies clients will need to collect the pk/vks, that's acceptable.

Copilot · 2026-01-30T04:07:55Z

@iamalwaysuncomfortable I've opened a new pull request, #3134, to work on those changes. Once the pull request is ready, I'll request review from you.

vicsn · 2026-01-30T09:23:32Z

I'd like to proceed with this, would you suggest that Konstantin and I take on the role of carrying this to the finish line

@iamalwaysuncomfortable @kpandl sounds good, we'll await your instructions for next steps, as you have context on the SDK side of things.

iamalwaysuncomfortable

@iamalwaysuncomfortable @kpandl sounds good, we'll await your instructions for next steps, as you have context on the SDK side of things.

Next week I'll ensure we address two separate concerns that preclude usage of this PR by the wallet.

Issue 1: Power Downloads

No caching or external storage available without file systems: No external storage exists in wasm and certain other systems like secure enclaves. This means both that the SRS can only be stored in memory currently and the only way to do this is with an internet connection (which is not always available). This means for EVERY Aleo web app, it must download hundreds of MB potentially for each usage of the web app.
Deprecrated Synchronous XmlHttpDownloads Required: Currently all parameter downloads are through this deprecated function (which is at potential risk of being unsupported entirely in future browsers) 2X the size of any downloads to be allocated due to the need to first change to a text-based encoding of the bytes first.

Issue 2: CommitterKey Population

Because this PR shifts the concern of SRS updates to the update function instead of importing SRS pieces from the Proving key. Any offline transactions on the web or environments where Sync XmlHttpRequests fail to work will lose the ability to prove.

Potential solutions:

I see the following solutions solve the above two problems, we'll make these PRs hopefully over the next two weeks.

Power Download Estimation (+ Optional Download): Functions that take in a group of proving key and verifying keys that specify the powers which must be inserted into the SRS and whether they currently exist in the SRS. These should optionally download the parameters asynchronously for clients who desire it.
External parameter insertion: Functions that allow parameters to be inserted externally so that after estimation, the executor can download the parameters (or fetch them from local storage) and insert them. This will ensure the wallet does not have continually download powers. These should be verified by checksums when inserted.
Constraint Estimation for Functions in Development: In the case where a function that has NOT yet been synthesized (i.e. developers making new functions) and no vk is available, a function that builds the circuit once and gets the circuit info as a first step should be developed.

Once we have these in place, we can carry this PR the rest of the way.

iamalwaysuncomfortable · 2026-01-30T19:56:53Z

A secondary note on what the result of the above proposal + this PR for the wallet.

Once this is in, wallets will be able to do the following:

Store 100s of proving keys instead of only 10-20.
Store the required powers locally and load them locally.
Avoid OOM issues that appear as a result of the sync XmlHttpRequest downloads.

Currently most wallets synthesize keys every time and the synthesis + download i/o is a significant portion of the time. This PR will significantly boost the local proving story.

ljedrz added 3 commits January 27, 2026 16:54

perf: use a singe committer_key and fft_precomputation for batch proving

6bc67a0

Signed-off-by: ljedrz <[email protected]>

perf: carry along an updateable universal_prover during synthesis and…

ee03ab5

… execution Signed-off-by: ljedrz <[email protected]>

chore: adjust the lockfile

750e041

Signed-off-by: ljedrz <[email protected]>

ljedrz requested a review from vicsn January 28, 2026 15:32

ljedrz added 2 commits January 29, 2026 10:31

fix: make CommitterKey & CircuitProvingKey serialization backward com…

793bb96

…patible Signed-off-by: ljedrz <[email protected]>

make CommitterKey & CircuitProvingKey objects backward compatible

8083480

Signed-off-by: ljedrz <[email protected]>

vicsn requested review from iamalwaysuncomfortable and kpandl January 29, 2026 18:52

iamalwaysuncomfortable reviewed Jan 30, 2026

View reviewed changes

Copilot AI mentioned this pull request Jan 30, 2026

Add periods to doc comments in modified files #3134

Draft

iamalwaysuncomfortable reviewed Jan 30, 2026

View reviewed changes

	/// This is `None` if `self` does not support lagrange bases
	/// This is `None` if `self` does not support lagrange bases.


		use std::collections::BTreeSet;

		#[derive(Clone, Debug, Default)]

	#[derive(Clone, Debug, Default)]
	/// Degree info specifies information needed to determine which powers to use to build the CommitterKey from the SRS.
	#[derive(Clone, Debug, Default)]

		let degree_diff = highest_degree_bound - new_highest_degree_bound;
		shifted_powers_of_beta_g = shifted_powers_of_beta_g.drain(degree_diff - 1..).collect_vec();

[Perf] A revival of 1955 & 2145 #3127

Are you sure you want to change the base?

[Perf] A revival of 1955 & 2145 #3127

Uh oh!

Conversation

ljedrz commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

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!

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!

Copilot AI commented Jan 30, 2026

Uh oh!

vicsn commented Jan 30, 2026

Uh oh!

iamalwaysuncomfortable left a comment

Choose a reason for hiding this comment

Issue 1: Power Downloads

Issue 2: CommitterKey Population

Potential solutions:

Uh oh!

iamalwaysuncomfortable commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ljedrz commented Jan 28, 2026 •

edited

Loading