fix: challenge generation update

barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp

@@ -79,24 +79,35 @@ template <typename FF> class BaseTranscript {
     TranscriptManifest manifest;
 
     /**
-     * @brief Compute c_next = H( Compress(c_prev || round_buffer) )
-     *
+     * @brief Compute next challenge c_next = H( Compress(c_prev || round_buffer) )
+     * @details This function computes a new challenge for the current round using the previous challenge
+     *          and the current round data, if they are exist.
+     *          It clears the current_round_data if nonempty after computing the challenge to minimize how much we
+     *          compress. It also sets previous_challenge_buffer to the current challenge buffer to set up next function
+     * call.
      * @return std::array<uint8_t, HASH_OUTPUT_SIZE>
      */
-    [[nodiscard]] std::array<uint8_t, HASH_OUTPUT_SIZE> get_next_challenge_buffer() const
+    [[nodiscard]] std::array<uint8_t, HASH_OUTPUT_SIZE> get_next_challenge_buffer()
     {
-        // Prevent challenge generation if nothing was sent by the prover.
-        ASSERT(!current_round_data.empty());
-
-        // concatenate the hash of the previous round (if not the first round) with the current round data.
+        // Empty buffer to compare against
+        std::array<uint8_t, HASH_OUTPUT_SIZE> empty_challenge_buffer{};
+        bool no_previous_challenge = (previous_challenge_buffer == empty_challenge_buffer);
+        // Prevent challenge generation if nothing was sent by the prover AND this is the first challenge we're
+        // generating.
+        ASSERT(!(current_round_data.empty() && no_previous_challenge));
+
+        // concatenate the previous challenge (if this is not the first challenge) with the current round data.
         // TODO(Adrian): Do we want to use a domain separator as the initial challenge buffer?
         // We could be cheeky and use the hash of the manifest as domain separator, which would prevent us from having
         // to domain separate all the data. (See https://safe-hash.dev)
         std::vector<uint8_t> full_buffer;
-        if (round_number > 0) {
+        if (!no_previous_challenge) {
             full_buffer.insert(full_buffer.end(), previous_challenge_buffer.begin(), previous_challenge_buffer.end());
         }
-        full_buffer.insert(full_buffer.end(), current_round_data.begin(), current_round_data.end());
+        if (!current_round_data.empty()) {
+            full_buffer.insert(full_buffer.end(), current_round_data.begin(), current_round_data.end());
+            current_round_data.clear(); // clear the round data buffer since it has been used
+        }
 
         // Pre-hash the full buffer to minimize the amount of data passed to the cryptographic hash function.
         // Only a collision-resistant hash-function like Pedersen is required for this step.
@@ -109,7 +120,8 @@ template <typename FF> class BaseTranscript {
 
         std::array<uint8_t, HASH_OUTPUT_SIZE> new_challenge_buffer;
         std::copy_n(base_hash.begin(), HASH_OUTPUT_SIZE, new_challenge_buffer.begin());
-
+        // update previous challenge buffer for next time we call this function
+        previous_challenge_buffer = new_challenge_buffer;
         return new_challenge_buffer;
     };
 
@@ -131,8 +143,11 @@ template <typename FF> class BaseTranscript {
   public:
     /**
      * @brief After all the prover messages have been sent, finalize the round by hashing all the data and then create
-     * the number of requested challenges which will be increasing powers of the first challenge.  Finally, reset the
-     * state in preparation for the next round.
+     * the number of requested challenges.
+     * @details Challenges are generated by iteratively hashing over the previous challenge, using
+     * get_next_challenge_buffer().
+     *          TODO(#741): Optimizations for this function include generalizing type of hash, splitting hashes into
+     * multiple challenges.
      *
      * @param labels human-readable names for the challenges for the manifest
      * @return std::array<FF, num_challenges> challenges for this round.
@@ -145,26 +160,23 @@ template <typename FF> class BaseTranscript {
         manifest.add_challenge(round_number, labels...);
 
         // Compute the new challenge buffer from which we derive the challenges.
-        auto next_challenge_buffer = get_next_challenge_buffer();
 
         // Create challenges from bytes.
         std::array<FF, num_challenges> challenges{};
 
-        std::array<uint8_t, sizeof(FF)> field_element_buffer{};
-        std::copy_n(next_challenge_buffer.begin(), HASH_OUTPUT_SIZE, field_element_buffer.begin());
-
-        challenges[0] = from_buffer<FF>(field_element_buffer);
-
-        // TODO(#583): rework the transcript to have a better structure and be able to produce a variable amount of
-        // challenges that are not powers of each other
-        for (size_t i = 1; i < num_challenges; i++) {
-            challenges[i] = challenges[i - 1] * challenges[0];
+        // Generate the challenges by iteratively hashing over the previous challenge.
+        for (size_t i = 0; i < num_challenges; i++) {
+            auto next_challenge_buffer = get_next_challenge_buffer(); // get next challenge buffer
+            std::array<uint8_t, sizeof(FF)> field_element_buffer{};
+            // copy half of the hash to lower 128 bits of challenge
+            std::copy_n(next_challenge_buffer.begin(),
+                        HASH_OUTPUT_SIZE / 2,
+                        field_element_buffer.begin() + HASH_OUTPUT_SIZE / 2);
+            challenges[i] = from_buffer<FF>(field_element_buffer);
         }
 
         // Prepare for next round.
         ++round_number;
-        current_round_data.clear();
-        previous_challenge_buffer = next_challenge_buffer;
 
         return challenges;
     }

barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp

@@ -185,6 +185,34 @@ TEST_F(UltraTranscriptTests, VerifierManifestConsistency)
     }
 }
 
+/**
+ * @brief Check that multiple challenges can be generated and sanity check
+ * @details We generate 6 challenges that are each 128 bits, and check that they are not 0.
+ *
+ */
+TEST_F(UltraTranscriptTests, ChallengeGenerationTest)
+{
+    // initialized with random value sent to verifier
+    auto transcript = ProverTranscript<FF>::init_empty();
+    // test a bunch of challenges
+    auto challenges = transcript.get_challenges("a", "b", "c", "d", "e", "f");
+    // print challenges and check they are not 0
+    for (size_t i = 0; i < challenges.size(); ++i) {
+        info("Challenge ", i, ": ", challenges[i]);
+        ASSERT_NE(challenges[i], 0) << "Challenge " << i << " is 0";
+    }
+    constexpr uint32_t random_val{ 17 }; // arbitrary
+    transcript.send_to_verifier("random val", random_val);
+    // test more challenges
+    auto [a, b, c] = transcript.get_challenges("a", "b", "c");
+    info("Challenge a: ", a);
+    info("Challenge b: ", b);
+    info("Challenge c: ", c);
+    ASSERT_NE(a, 0) << "Challenge a is 0";
+    ASSERT_NE(b, 0) << "Challenge a is 0";
+    ASSERT_NE(b, 0) << "Challenge a is 0";
+}
+
 TEST_F(UltraTranscriptTests, FoldingManifestTest)
 {
     using Flavor = flavor::Ultra;