chore: audit Permutation argument 3 #18657

notnotraju · 2025-11-27T17:29:48Z

🧾 Audit Context

Part of the (generalized) permutation audit.

🛠️ Changes Made

Moved around/improved testing
Added documentation

✅ Checklist

Audited all methods of the relevant module/class
Audited the interface of the module/class with other (relevant) components
Documented existing functionality and any changes made (as per Doxygen requirements)
Resolved and/or closed all issues/TODOs pertaining to the audited files
Confirmed and documented any security or other issues found (if applicable)
Verified that tests cover all critical paths (and added tests if necessary)
Updated audit tracking for the files audited (check the start of each file you audited)

📌 Notes for Reviewers

The diff misleadingly large because some tests were migrated and some boilerplate changed.

notnotraju · 2025-11-27T17:31:29Z

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp

+    void set_tau_at_index(const uint32_t tag_index, const uint32_t tau_index)
    {
        this->_tau.insert({ tag_index, tau_index });
-        this->current_tag++; // Why exactly?


unnecessary, as long as we only access tags by the get_new_tag() method.

notnotraju · 2025-11-27T17:32:12Z

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp

+     * @param tag_index_2
+     * @return uint32_t
+     */
+    void set_tau_transposition(const uint32_t tag_index_1, const uint32_t tag_index_2)


our $\tau$ is the product of disjoint transpositions (i.e., has order 1 or 2), hence this is universally what is used.

notnotraju · 2025-11-27T17:34:26Z

barretenberg/cpp/src/barretenberg/honk/composer/composer_lib.test.cpp

+ * circuit fails.
+ *
+ */
+TEST_F(ComposerLibTests, TagCollisionFailure)


test might not belong here, the "composer" directory seems deprecated, we might want to refactor and move stuff elsewhere for clarity. still, for the time being, worth testing the code in the directory in the same directory.

notnotraju · 2025-11-27T17:35:46Z

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp

+     * @param tau_index
+     * @return uint32_t
+     */
+    void set_tau_at_index(const uint32_t tag_index, const uint32_t tau_index)


create_tag is a bit misleading, what this does is set the $\tau(i)=j$.

notnotraju · 2025-12-02T22:00:48Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

This file is about half old tests, slightly rewritten.

notnotraju · 2025-12-02T22:01:20Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

+ * @note There are no tags/multiset-equality checks in this test.
+ * @note We exclude ZK flavors because we manually tamper with precomputed polynomials.
+ */
+TYPED_TEST(PermutationNonZKTests, SigmaCorruptionFailure)


new in this PR

notnotraju · 2025-12-02T22:01:33Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

+ *
+ * @note We exclude ZK flavors because we manually tamper with relation parameters.
+ */
+TYPED_TEST(PermutationNonZKTests, PublicInputDeltaMismatch)


new in this PR

notnotraju · 2025-12-02T22:06:21Z

barretenberg/cpp/src/barretenberg/honk/library/grand_product_library.hpp


 namespace bb {

-// TODO(luke): This contains utilities for grand product computation and is not specific to the permutation grand


notnotraju · 2025-12-02T22:07:10Z

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp

-     * @brief The permutation on variable tags
-     * @details See https://github.com/AztecProtocol/plonk-with-lookups-private/blob/new-stuff/GenPermuations.pdf
+     * @brief The permutation on variable tags, as a constituent of the generalized permutation argument.
+     * @details See S6 of https://github.com/AztecProtocol/plonk-with-lookups-private/blob/new-stuff/GenPermuations.pdf


link doesn't exist, not sure where we should permalink this.

(plookup paper doesn't have the relevant S6)

notnotraju · 2025-12-03T11:42:30Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

+ * the multiset of values with second_tag (after applying tau). Here both multisets are {x, y},
+ * so the check passes.
+ */
+TYPED_TEST(PermutationTests, NonTrivialTagPermutation)


notnotraju · 2025-12-03T11:42:39Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

+ * The test verifies that both mechanisms can coexist: the grand product argument must account for
+ * both the wire permutation cycles AND the tag-based multiset equality check.
+ */
+TYPED_TEST(PermutationTests, NonTrivialGeneralizedPerm)


notnotraju · 2025-12-03T11:42:48Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

+ * The second sub-test creates the same circuit WITHOUT tags to confirm that the circuit's
+ * arithmetic constraints are satisfied, proving the failure above is due to multiset-equality mismatch.
+ */
+TYPED_TEST(PermutationTests, BadTagPermutation)


notnotraju · 2025-12-03T11:43:07Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

+ * @note This test does not use `prove_and_verify`. Indeed, there is no straightforward way to tamper with z_perm,
+ * created after finalization, using `prove_and_verify`.
+ */
+TYPED_TEST(PermutationNonZKTests, ZPermZeroedOutFailure)


old test (earlier part of this audit)

notnotraju · 2025-12-03T11:43:19Z

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

+ * @note This test excludes ZK flavors because we manually tamper with z_perm, which would
+ * conflict with the ZK masking applied to witness polynomials.
+ */
+TYPED_TEST(PermutationNonZKTests, ZPermShiftNotZeroAtLagrangeLastFailure)


old test (earlier part of this audit)

ledwards2225

Good stuff!

ledwards2225 · 2025-12-03T15:16:20Z

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp

-     * real_variable_index[index] == index, i.e. it is the identity map
+     * real_variable_index[index] == index, i.e., it is the identity map.
+     *
+     * @note If there is a copy constraint between witness indices idx_a and idx_b, then their they will both point to


Suggested change

* @note If there is a copy constraint between witness indices idx_a and idx_b, then their they will both point to

* @note If there is a copy constraint between witness indices idx_a and idx_b, then they will both point to

ledwards2225 · 2025-12-03T15:20:26Z

barretenberg/cpp/src/barretenberg/trace_to_polynomials/trace_to_polynomials.hpp

-     * of space within the wire polynomials, regardless of how many actual constraints of each type exist. This is
-     * useful primarily for folding since it guarantees that the set of relations that must be executed at each row is
-     * consistent across all folding steps.
+     * @note By default, this method constructs an exectution trace that is sorted by gate type.


Suggested change

* @note By default, this method constructs an exectution trace that is sorted by gate type.

* @note By default, this method constructs an execution trace that is sorted by gate type.

ledwards2225 · 2025-12-03T15:25:08Z

barretenberg/cpp/src/barretenberg/honk/library/grand_product_library.hpp

 * where ∏ := ∏_{j=0:i-1}
 *
- * The specific algebraic relation used by Z_perm is defined by Flavor::GrandProductRelations
+ * (Note that Z_perm[1] may be thought of as the quotient of the empty product by the empty product, and hence setting


unfinished thought?

ledwards2225 · 2025-12-03T15:28:28Z

barretenberg/cpp/src/barretenberg/honk/library/grand_product_library.hpp

+ * optimization having to do with public inputs.)
 *
- * For example, in Flavor::Standard the relation describes:
+ * The following is the classic, no-public-inputs PLONK permutation argument on 4 wires. This bears witness to the


This seems a little bit misleading since this indeed the method we use to compute z_perm for the fully general case (public inputs + generalized perms), not just trad copy constraints. Maybe with this comment you're interpreting \simga and id as the unadulterated entities that only account for copy constraints rather than the sigma and id polynomials which account for everything?

notnotraju added 2 commits November 27, 2025 16:30

cleanup of some tag-based code

41bbcd0

add a tag failure test in ComposerLibTests, which does prove/verify.

0693cfe

notnotraju added ci-full Run all master checks. bberg-int-audit All things related to barretenberg internal audit ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure labels Nov 27, 2025

notnotraju commented Nov 27, 2025

View reviewed changes

notnotraju added 4 commits November 28, 2025 20:38

further documentation

46429bc

documentation

31a74e7

tests mildly refactored

cebbe15

revamped permutation tests

298bb0a

notnotraju commented Dec 2, 2025

View reviewed changes

barretenberg/cpp/src/barretenberg/ultra_honk/permutation.test.cpp

Copy link

Contributor Author

notnotraju Dec 2, 2025

Choose a reason for hiding this comment

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

This file is about half old tests, slightly rewritten.

notnotraju commented Dec 2, 2025

View reviewed changes

cleanup and detailed documentation

5e24bfc

notnotraju marked this pull request as ready for review December 3, 2025 11:41

notnotraju commented Dec 3, 2025

View reviewed changes

Merge branch 'merge-train/barretenberg' into rk/permutation-audit-3

9c61aba

notnotraju requested a review from ledwards2225 December 3, 2025 11:48

ledwards2225 approved these changes Dec 3, 2025

View reviewed changes


		namespace bb {

		// TODO(luke): This contains utilities for grand product computation and is not specific to the permutation grand

	* @note If there is a copy constraint between witness indices idx_a and idx_b, then their they will both point to
	* @note If there is a copy constraint between witness indices idx_a and idx_b, then they will both point to

	* @note By default, this method constructs an exectution trace that is sorted by gate type.
	* @note By default, this method constructs an execution trace that is sorted by gate type.

chore: audit Permutation argument 3 #18657

Are you sure you want to change the base?

chore: audit Permutation argument 3 #18657

Conversation

notnotraju commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧾 Audit Context

🛠️ Changes Made

✅ Checklist

📌 Notes for Reviewers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

notnotraju Nov 27, 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!

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!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

notnotraju commented Nov 27, 2025 •

edited

Loading

notnotraju Nov 27, 2025 •

edited

Loading