Skip to content

feat!: Merge with degree check#15562

Merged
federicobarbacovi merged 118 commits into
merge-train/barretenbergfrom
fb/merge_with_degree_check
Jul 15, 2025
Merged

feat!: Merge with degree check#15562
federicobarbacovi merged 118 commits into
merge-train/barretenbergfrom
fb/merge_with_degree_check

Conversation

@federicobarbacovi

@federicobarbacovi federicobarbacovi commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

We modify the Merge protocol so that it enforces that the subtable polynomial t_j has degree smaller than subtable_size as read from the proof.

Closes AztecProtocol/barretenberg#1442

Details

As per the linked issue, we want to support ecc operations in app circuits. To ensure that app ecc ops do not modify ecc ops that happened before them, we need to ensure that the subtable length t.size() is smaller than the constant l by which we right shift T_prev. This is to ensure that t + X^{l-1} T_prev is indeed the polynomial corresponding to the column t || T_prev.

We enforce this degree check in the merge protocol by requiring the prover to commit to g(X) := X^{l-1} t(1/X) and provide openings c, d of t, g at challenges 1/kappa, kappa, respectively, for which we check c * kappa^{l-1} = d.

To save on the number of MSMs performed, we use Shplonk to verify the following claims:

  • t(X) opens to c at 1/kappa
  • p(X) = t(X) + X^{l-1} T_prev(X) - T(X) opens to 0 at kappa
  • g(X) := X^{l-1} t(1/X) opens to d at kappa

AztecBot and others added 22 commits July 4, 2025 02:11
* Our basic binary ops produce a new gate iff both operands are
witnesses. Most of the implementations contained deeply branched logic
that is now replaced with simple `int` arithmetic that looks more robust
and is much easier to read.

* Added documentation in various places. The constraints are made more
explicit
* Removed redundant assignments
* Re-worked the test suite. Now all basic ops and functions are tested
for all combinations of flags (`is_constant`, `witness_bool`,
`witness_inverted`). Added missing tests, checked various edge cases.
* Removed a couple of tests that seemed less expressive
* Removed `must_imply()` method for a vector of implications, as it is
not used anymore
* Fixed a `normalize()` regression - previously it wouldn't check if
(`witness_inverted == false`) and would normalize a witness `bool_t`
even if the condition is satisfied. It is the reason for the VK changes
We enhance the Shplonk verifier api so that it can efficiently handle
openings of polynomials that are linearly dependent

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com>
The `bit_array` stdlib primitive is not used in the production code -->
deleted

Had to change a couple of headers that included `bit_array` to
access`plookup` or `uint`.

Fixed some misleading comments in a couple of fuzzers
`ECCVM-` and `Translator-` `RecursiveFlavors` are only used with
`UltraCircuitBuilder`, which allows us to decouple them and the
corresponding `RecursiveVerifiers` from `Flavor` template parameter.
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..)

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
The tube propagates the public inputs of the hiding circuit (received in
the CIVC proof) via its own public inputs. Previously, those public
inputs were being extracted from the native proof meaning they were
disconnected from those used by the CIVC recursive verifier. A previous
PR made the CIVC rec verifier take a stdlib proof so it is now easy to
directly set the pub inputs of the stdlib proof public, making the
connection proper.

Partially (entirely?) addresses
AztecProtocol/barretenberg#1048
Addresses
[1140](AztecProtocol/barretenberg#1140),
native IPA verifier computes inverses of the challenges via a
`batch_invert()` call.

---------

Co-authored-by: notnotraju <raju@aztec-labs.com>
Moves the `uint_plookup` class into the `uint` class in stdlib. This is
done because the plookup-based uint version is now the default (since we
no more the old `standard` or `turbo` arithmetisation).

As a result, we remove any conditionals using `HasPlookup` from uint
related code.
CommitmentKey commitment_key,
const std::shared_ptr<Transcript>& transcript)
: op_queue(op_queue)
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1420): pass commitment keys by value

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue is closed

@federicobarbacovi federicobarbacovi self-assigned this Jul 8, 2025
@federicobarbacovi federicobarbacovi marked this pull request as ready for review July 8, 2025 12:52
I had thought that const ref can't get a default parameter but it can!
@AztecBot AztecBot force-pushed the merge-train/barretenberg branch from 42b65c1 to 8369b3f Compare July 8, 2025 14:03
}
}

TYPED_TEST(MegaHonkTests, MultipleCircuitsMergeOnlyAppend)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed as discussed (it would fail because Shplonk doesn't handle empty polynomials #1474 and in an append-only scenario the table to be prepended is the empty table, hence the empty polynomial)

Base automatically changed from merge-train/barretenberg to next July 14, 2025 14:40

@maramihali maramihali left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the modifications, another bulk of comments but mostly nits! :)

Comment thread barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.hpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp
@maramihali maramihali changed the base branch from next to merge-train/barretenberg July 14, 2025 15:11
Base automatically changed from merge-train/barretenberg to next July 15, 2025 05:41
@federicobarbacovi federicobarbacovi changed the base branch from next to merge-train/barretenberg July 15, 2025 08:04

@maramihali maramihali left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for bearing through my endless review :D

Comment thread barretenberg/cpp/src/barretenberg/ultra_honk/merge_verifier.cpp Outdated
std::string suffix = std::to_string(idx);
T_prev_commitments[idx] = transcript->template receive_from_prover<Commitment>("T_PREV_" + suffix);
T_commitments[idx] = transcript->template receive_from_prover<Commitment>("T_CURRENT_" + suffix);
if (settings == MergeSettings::PREPEND) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right, so this if clause will go away once issue 1473 is resolved

@federicobarbacovi federicobarbacovi merged commit 0772339 into merge-train/barretenberg Jul 15, 2025
4 checks passed
@federicobarbacovi federicobarbacovi deleted the fb/merge_with_degree_check branch July 15, 2025 10:00
@federicobarbacovi federicobarbacovi restored the fb/merge_with_degree_check branch July 15, 2025 10:24
johnathan79717 added a commit that referenced this pull request Jul 15, 2025
Restore accidentally deleted files in a previous PR.
#15562
@federicobarbacovi federicobarbacovi deleted the fb/merge_with_degree_check branch July 15, 2025 11:11
github-merge-queue Bot pushed a commit that referenced this pull request Jul 16, 2025
See
[merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md).

BEGIN_COMMIT_OVERRIDE
feat!: Merge with degree check (#15562)
fix: Fix the docker build action for fuzzing (#15719)
fix: restore accidentally deleted files (#15724)
fix: civc wasm memory regression (#15722)
feat: mmap backed polynomials (#15531)
feat(bbapi): CLI uses bbapi CIVC (#15702)
fix(ci): brittle benchmark behavior (#15771)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: ludamad <adam.domurad@gmail.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>
Co-authored-by: ludamad <domuradical@gmail.com>
Co-authored-by: maramihali <mara@aztecprotocol.com>
Co-authored-by: Sarkoxed <75146596+Sarkoxed@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants