From f63e7a94b1ba555eecbe08b7114e8b6ad0b82bc0 Mon Sep 17 00:00:00 2001
From: Lucas Xia <lucasxia01@gmail.com>
Date: Thu, 1 Feb 2024 15:08:40 -0500
Subject: [PATCH] refactor: poseidon2 hash uses span instead of vector (#4003)

Updating the hash() function in poseidon2 to take in a std::span instead
of a std::vector, and avoiding a needless copy.

Also updates poseidon2 hash_buffer test to check an inequivalent hash.
---
 .../src/barretenberg/crypto/poseidon2/poseidon2.cpp   |  3 +--
 .../barretenberg/crypto/poseidon2/poseidon2.test.cpp  | 11 +++++++----
 .../barretenberg/crypto/poseidon2/sponge/sponge.hpp   |  7 ++++---
 .../barretenberg/stdlib/hash/poseidon2/poseidon2.cpp  |  3 +--
 .../stdlib/hash/poseidon2/sponge/sponge.hpp           |  6 +++---
 5 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.cpp b/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.cpp
index 1fce753975b..635b8dc0896 100644
--- a/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.cpp
+++ b/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.cpp
@@ -7,8 +7,7 @@ namespace bb::crypto {
 template <typename Params>
 typename Poseidon2<Params>::FF Poseidon2<Params>::hash(const std::vector<typename Poseidon2<Params>::FF>& input)
 {
-    auto input_span = input;
-    return Sponge::hash_fixed_length(input_span);
+    return Sponge::hash_fixed_length(input);
 }
 
 /**
diff --git a/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.test.cpp
index 7903d1fceb6..c18da87b739 100644
--- a/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.test.cpp
+++ b/barretenberg/cpp/src/barretenberg/crypto/poseidon2/poseidon2.test.cpp
@@ -52,12 +52,15 @@ TEST(Poseidon2, HashBufferConsistencyCheck)
     // element
     fr a(std::string("00000b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"));
 
-    auto input_vec = to_buffer(a);      // takes field element and converts it to 32 bytes
+    // takes field element and converts it to 32 bytes
+    auto input_vec = to_buffer(a);
+    bb::fr result1 = crypto::Poseidon2<crypto::Poseidon2Bn254ScalarFieldParams>::hash_buffer(input_vec);
     input_vec.erase(input_vec.begin()); // erase first byte since we want 31 bytes
+    fr result2 = crypto::Poseidon2<crypto::Poseidon2Bn254ScalarFieldParams>::hash_buffer(input_vec);
+
     std::vector<fr> input{ a };
     auto expected = crypto::Poseidon2<crypto::Poseidon2Bn254ScalarFieldParams>::hash(input);
 
-    fr result = crypto::Poseidon2<crypto::Poseidon2Bn254ScalarFieldParams>::hash_buffer(input_vec);
-
-    EXPECT_EQ(result, expected);
+    EXPECT_NE(result1, expected);
+    EXPECT_EQ(result2, expected);
 }
diff --git a/barretenberg/cpp/src/barretenberg/crypto/poseidon2/sponge/sponge.hpp b/barretenberg/cpp/src/barretenberg/crypto/poseidon2/sponge/sponge.hpp
index 2a99b88609f..eb89c96e471 100644
--- a/barretenberg/cpp/src/barretenberg/crypto/poseidon2/sponge/sponge.hpp
+++ b/barretenberg/cpp/src/barretenberg/crypto/poseidon2/sponge/sponge.hpp
@@ -129,7 +129,8 @@ template <typename FF, size_t rate, size_t capacity, size_t t, typename Permutat
      * @param input
      * @return std::array<FF, out_len>
      */
-    template <size_t out_len, bool is_variable_length> static std::array<FF, out_len> hash_internal(std::span<FF> input)
+    template <size_t out_len, bool is_variable_length>
+    static std::array<FF, out_len> hash_internal(std::span<const FF> input)
     {
         size_t in_len = input.size();
         const uint256_t iv = (static_cast<uint256_t>(in_len) << 64) + out_len - 1;
@@ -153,11 +154,11 @@ template <typename FF, size_t rate, size_t capacity, size_t t, typename Permutat
         return output;
     }
 
-    template <size_t out_len> static std::array<FF, out_len> hash_fixed_length(std::span<FF> input)
+    template <size_t out_len> static std::array<FF, out_len> hash_fixed_length(std::span<const FF> input)
     {
         return hash_internal<out_len, false>(input);
     }
-    static FF hash_fixed_length(std::span<FF> input) { return hash_fixed_length<1>(input)[0]; }
+    static FF hash_fixed_length(std::span<const FF> input) { return hash_fixed_length<1>(input)[0]; }
 
     template <size_t out_len> static std::array<FF, out_len> hash_variable_length(std::span<FF> input)
     {
diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2.cpp
index d113d2a3cb8..705493bf345 100644
--- a/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2.cpp
+++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2.cpp
@@ -14,8 +14,7 @@ template <typename C> field_t<C> poseidon2<C>::hash(C& builder, const std::vecto
      * This should just call the sponge variable length hash function
      *
      */
-    auto input{ inputs };
-    return Sponge::hash_fixed_length(builder, input);
+    return Sponge::hash_fixed_length(builder, inputs);
 }
 
 /**
diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp
index bccebbd152c..6d79834aa95 100644
--- a/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp
+++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp
@@ -135,7 +135,7 @@ template <size_t rate, size_t capacity, size_t t, typename Permutation, typename
      * @return std::array<field_t, out_len>
      */
     template <size_t out_len, bool is_variable_length>
-    static std::array<field_t, out_len> hash_internal(Builder& builder, std::span<field_t> input)
+    static std::array<field_t, out_len> hash_internal(Builder& builder, 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;
@@ -160,11 +160,11 @@ template <size_t rate, size_t capacity, size_t t, typename Permutation, typename
     }
 
     template <size_t out_len>
-    static std::array<field_t, out_len> hash_fixed_length(Builder& builder, std::span<field_t> input)
+    static std::array<field_t, out_len> hash_fixed_length(Builder& builder, std::span<const field_t> input)
     {
         return hash_internal<out_len, false>(builder, input);
     }
-    static field_t hash_fixed_length(Builder& builder, std::span<field_t> input)
+    static field_t hash_fixed_length(Builder& builder, std::span<const field_t> input)
     {
         return hash_fixed_length<1>(builder, input)[0];
     }