chore: `stdlib::poseidon2` internal audit by iakovenkos · Pull Request #16534 · AztecProtocol/aztec-packages

iakovenkos · 2025-08-22T15:38:38Z

🧾`stdlib::poseidon2` internal audit

Most of the Poseidon2-related methods and classes were unnecessarily templated on a variety of parameters, such as rate and capacity of the permutation, while at the same time our Circuit Builders would only support the case rate = 3, capacity = 1. To avoid confusion and improve the readability, I minimized the number of templates.
The sponge class was only invoked with a sequence of absorb calls that would lead to a single squezze call. It allowed to simplify the class logic.
Simplified the Poseidon2Permutation class by re-using some of the field_t methods. Improved the docs.
Added a detailed readme that contains links to the sources, including the padding choices in sponge and the explanation of the custom Poseidon2 gates
Audited relations, expanded corresponding docs
Added a bunch of tests - collision-resistance sanity check, test against the hash outputs from https://github.com/zemse/poseidon2-evm, several basic failure tests

…udit

iakovenkos · 2025-08-25T18:14:54Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2_permutation.hpp

 namespace bb::stdlib {

-template <typename Params, typename Builder> class Poseidon2Permutation {
+template <typename Builder> class Poseidon2Permutation {


redundant template param

iakovenkos · 2025-08-25T18:17:24Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2_permutation.cpp

+    const bb::fr four(4);
    // create the 6 gates for the initial matrix multiplication
    // gate 1: Compute tmp1 = state[0] + state[1] + 2 * state[3]
-    field_t<Builder> tmp1 =


seems more robust to re-use add_two from field_t, as we don't allow hashing constants, the result of add_two is always a new witness, which is normalized. hence, it's safe to use get_witness_index() above.

iakovenkos · 2025-08-25T18:18:46Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

-template <size_t rate, size_t capacity, size_t t, typename Permutation, typename Builder> class FieldSponge {
+template <size_t rate, size_t capacity, size_t t, typename Builder> class FieldSponge {
  public:
-    /**


Not needed in our use-cases.

are we opting to fix the capacity etc?

iakovenkos · 2025-08-25T18:20:27Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

    }

-    std::array<field_t, rate> perform_duplex()
+    void perform_duplex()


this method used to mutate state and return the output equal to the updated state.

iakovenkos · 2025-08-25T18:22:28Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

     * @param input
     * @return std::array<field_t, out_len>
     */
-    template <size_t out_len>


the function is only used with out_len == 1

iakovenkos · 2025-08-29T15:19:59Z

barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp

        ASSERT(!data.empty());
-        ASSERT(data[0].get_context() != nullptr);

-        Builder* builder = data[0].get_context();


this is done in the hash method

…i/poseidon-2-internal-audit

...src/barretenberg/boomerang_value_detection/graph_description_poseidon2s_permutation.test.cpp

barretenberg/cpp/src/barretenberg/dsl/acir_format/poseidon2_constraint.cpp

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

barretenberg/cpp/src/barretenberg/polynomials/univariate.hpp

barretenberg/cpp/src/barretenberg/relations/poseidon2_external_relation.hpp

barretenberg/cpp/src/barretenberg/relations/poseidon2_internal_relation.hpp

kashbrti · 2025-09-04T10:31:38Z

barretenberg/cpp/src/barretenberg/relations/poseidon2_internal_relation.hpp

+        // Add ĉ₀⁽ⁱ⁾ stored in the selector and convert to Lagrange basis
+        auto s1 = Accumulator(w_1 + c_0_int);
+
+        // Apply S-box. Note that the multiplication is performed point-wise


hmm, should we add an S-BOX method at this point instead of having the macro and use it across both internal and external relation?

I don't knoow, feels a bit redundant tbh, cause it's only done once in the internal one

barretenberg/cpp/src/barretenberg/relations/poseidon2_internal_relation.hpp

kashbrti

other than few nitpicks lgtm. haven't looked too much into the README and the tests.

kashbrti · 2025-09-04T10:47:23Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

-template <size_t rate, size_t capacity, size_t t, typename Permutation, typename Builder> class FieldSponge {
+template <size_t rate, size_t capacity, size_t t, typename Builder> class FieldSponge {
  public:
-    /**


are we opting to fix the capacity etc?

kashbrti · 2025-09-04T10:50:12Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

-    };
+template <typename Builder> class FieldSponge {
+  private:
+    using Permutation = Poseidon2Permutation<Builder>;


could you add a reference where we have gotten the sponge spec and the parameters from?

it's in the readme

kashbrti · 2025-09-04T10:56:08Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

        state = Permutation::permutation(builder, state);
-        // return `rate` number of field elements from the sponge state.
-        std::array<field_t, rate> output;
-        for (size_t i = 0; i < rate; ++i) {


were we just copying the state and outputting it here?

kashbrti · 2025-09-04T11:10:30Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

+    static field_t hash_internal(std::span<const field_t> input)
    {
-        size_t in_len = input.size();
-        const uint256_t iv = (static_cast<uint256_t>(in_len) << 64) + out_len - 1;


just moved to the constructor. still have mixed feelings about the amount of flexibility we're removing from the sponge. I guess internally it doesn't matter. but thinking whether we want it to be a standalone primitive that other devs can play around with.

there was no flexibility tbh, cause create_poseidon2_***_gate supports only the case where rate + capacity = num_wires (=4). templating those circuit builder methods on these params and storing several batches of round constants sounds painful. so before my changes we couldn't just instantiate the permutation with different params

kashbrti · 2025-09-04T11:14:54Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

+        // The final state consists of 4 elements, we only use the first element, which means that the remaining
+        // 3 witnesses are only used in a single gate.
+        for (const auto& elem : sponge.state) {
+            builder->update_used_witnesses(elem.witness_index);


this used to be skipped for MegaCircuitBuilder was there a reason for this?

Daniel's boomerang test is only instantiated with UltraCircuitBuilder but in both cases the "tail" of the final state is un-used

...src/barretenberg/boomerang_value_detection/graph_description_poseidon2s_permutation.test.cpp

kashbrti · 2025-09-04T13:26:54Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2_permutation.hpp

-    static void matrix_multiplication_external(Builder* builder, State& state);
+    static void record_current_state_into_next_row(Builder* builder, const State& state, auto& block)
+    {
+        builder->create_dummy_gate(block,


create_dummy_gate just constructs a gate that is never active? I agree then that the naming is funky

kashbrti · 2025-09-04T13:31:54Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2_permutation.cpp

-template <typename Params, typename Builder>
-typename Poseidon2Permutation<Params, Builder>::State Poseidon2Permutation<Params, Builder>::permutation(
-    Builder* builder, const typename Poseidon2Permutation<Params, Builder>::State& input)
+template <typename Builder>


just fixing the params to BN254 it seems.

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2_permutation.cpp

kashbrti · 2025-09-04T13:38:41Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2_permutation.hpp

+     * @param block Either `poseidon2_external` or `poseidon2_internal` block of the Execution Trace
     */
-    static void matrix_multiplication_external(Builder* builder, State& state);
+    static void record_current_state_into_next_row(Builder* builder, const State& state, auto& block)


nitpick, but maybe store or write is a better name? (write might be confused by a memory write)

changed to propagate_current_state_to_next_row()

iakovenkos · 2025-09-04T14:03:26Z

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp

-        // add the cache into sponge state
+        // Add the cache into sponge state
        for (size_t i = 0; i < rate; ++i) {
            state[i] += cache[i];


@kashbrti the padding is happening here. when squeeze calls perform_duplex(), only first num_inputs % 3 are not const 0

hmm, so I see that we're keeping a cache of size 3, whenever it fills up we're calling the perform_duplex. don't see what we do when for the last block (of length 3) we don't have more elements to fill the block. I guess in this case, we're just calling squeeze on it?

squeeze will again call perform_duplex. but the cache will not have an element in index 2 anymore, am I missing something? because it seems each call of perform_duplex just empties the cache.

ah cache is a std::array<field_t,3> and not a std::vector<field_t> so it'll have a 0 field_t there.
but shouldn't we have a bit of 1 before starting to pad with 0s? like now we will have a collision between a genuine input that ends with 0 and one that needs padding by 1 element it seems.

Well, we don't support fixed length mode at all. So, IV prevents this. I added a test checking that there are no collisions between {x}, {x,0}, and {x,0,0}

I guess the length used in the IV would mitigate that, but still might be better to take a standard approach.

ah seems like I forgot to press comment XD. yea I guess the IV is preventing the collisions

kashbrti · 2025-09-05T10:07:18Z

also please add a PR description.

…i/poseidon-2-internal-audit

BEGIN_COMMIT_OVERRIDE chore: databus internal audit - small cleanups and tests coverage (#16703) chore: `stdlib::poseidon2` internal audit (#16534) END_COMMIT_OVERRIDE

- Most of the `Poseidon2`-related methods and classes were unnecessarily templated on a variety of parameters, such as `rate` and `capacity` of the permutation, while at the same time our Circuit Builders would only support the case `rate = 3`, `capacity = 1`. To avoid confusion and improve the readability, I minimized the number of templates. - The `sponge` class was only invoked with a sequence of `absorb` calls that would lead to a single `squezze` call. It allowed to simplify the class logic. - Simplified the `Poseidon2Permutation` class by re-using some of the `field_t` methods. Improved the docs. - Added a detailed **readme** that contains links to the sources, including the padding choices in `sponge` and the explanation of the custom Poseidon2 gates - Audited relations, expanded corresponding docs - Added a bunch of tests - collision-resistance sanity check, test against the hash outputs from https://github.com/zemse/poseidon2-evm, several basic failure tests

### 🧾`stdlib::poseidon2` internal audit - Most of the `Poseidon2`-related methods and classes were unnecessarily templated on a variety of parameters, such as `rate` and `capacity` of the permutation, while at the same time our Circuit Builders would only support the case `rate = 3`, `capacity = 1`. To avoid confusion and improve the readability, I minimized the number of templates. - The `sponge` class was only invoked with a sequence of `absorb` calls that would lead to a single `squezze` call. It allowed to simplify the class logic. - Simplified the `Poseidon2Permutation` class by re-using some of the `field_t` methods. Improved the docs. - Added a detailed **readme** that contains links to the sources, including the padding choices in `sponge` and the explanation of the custom Poseidon2 gates - Audited relations, expanded corresponding docs - Added a bunch of tests - collision-resistance sanity check, test against the hash outputs from https://github.com/zemse/poseidon2-evm, several basic failure tests

iakovenkos added 8 commits August 21, 2025 14:34

simplify

d5206c4

witness_idx -> get_witness_index()

d6ba445

remove Params template from Poseidon2 Permutation

4cb6eb8

edit relation docs

068b82a

Merge branch 'merge-train/barretenberg' into si/poseidon-2-internal-a…

8f74eec

…udit

fix types

554eb12

rm unnecessary var

3e6405f

extend poseidon2 ext/int relation docs

0e06803

iakovenkos self-assigned this Aug 25, 2025

iakovenkos added 3 commits August 25, 2025 18:04

fix

e1473e7

simplify sponge

b3d14d5

remove modes from sponge

fd15106

iakovenkos commented Aug 25, 2025

View reviewed changes

iakovenkos added 13 commits August 26, 2025 10:31

simplify further

cf1aea0

add a helper to record state

338e7b0

permutation docs

23b70bf

sponge clean-up

59446e2

remove unnecessary 0 witnesses

934a9d7

update used witnesses only once

b611f21

comment about boomerang + remove template param

762a979

padding with constant zeroes

ce15454

add readme

64b8b7a

remove builder from hash args

49e321d

cleaner inclusions, remove manual assert_equal gate from acir

26e11aa

test against independent values

94c4c07

add num gates pinning

fabc32f

iakovenkos commented Aug 29, 2025

View reviewed changes

iakovenkos added 5 commits August 29, 2025 16:51

add poseidon2 circuit failure test

402d1e7

modify tests

b866401

clean up the the failure test

60185cb

Merge remote-tracking branch 'origin/merge-train/barretenberg' into s…

3554c7e

…i/poseidon-2-internal-audit

vks update

f9dabf6

iakovenkos marked this pull request as ready for review September 3, 2025 13:06

iakovenkos requested review from IlyasRidhuan, fcarreiro and jeanmon as code owners September 3, 2025 13:06

iakovenkos added 3 commits September 3, 2025 13:47

fix fuzzing build

d98193c

fix tests

6713859

vks update

84d218e

fcarreiro removed their request for review September 3, 2025 15:38

iakovenkos requested a review from kashbrti September 3, 2025 15:45

kashbrti reviewed Sep 4, 2025

View reviewed changes

kashbrti approved these changes Sep 4, 2025

View reviewed changes

iakovenkos commented Sep 4, 2025

View reviewed changes

iakovenkos added 3 commits September 5, 2025 10:53

resolve review comments

933a0c9

Merge remote-tracking branch 'origin/merge-train/barretenberg' into s…

c9ad083

…i/poseidon-2-internal-audit

update num gates in megazkrecursive

cd4c136

iakovenkos changed the title ~~chore: stdlib poseidon2 internal audit~~ chore: stdlib::poseidon2 internal audit Sep 5, 2025

update vks

1f3e610

iakovenkos merged commit 41de29a into merge-train/barretenberg Sep 5, 2025
6 checks passed

iakovenkos deleted the si/poseidon-2-internal-audit branch September 5, 2025 11:45

AztecBot mentioned this pull request Sep 5, 2025

feat: merge-train/barretenberg #16805

Merged

Conversation

iakovenkos commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧾stdlib::poseidon2 internal audit

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!

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!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

kashbrti commented Sep 5, 2025

Uh oh!

Uh oh!

iakovenkos commented Aug 22, 2025 •

edited

Loading

🧾`stdlib::poseidon2` internal audit