fix: challenge generation update (#2628)

This PR updates challenge generation to not be powers of each other,
fixing AztecProtocol/barretenberg#583.
Before, we generated multiple challenges by generating one by hashing
the round data and the previous challenge, and then taking powers of
this challenge as the other challenges. Now, we generate challenges by
taking a hash each time of the previous challenge (and the round data,
which is only nonempty in the first iteration of get_next_challenge_buffer()
in a round).

Using powers of one challenge does not lead to soundness bugs in how we
currently use challenges, but theoretically, we would want challenges to
be "independent" of each other.

Challenges are 128 bits. I truncate every 256-bit hash to 128 bits for 2
reasons. First, it is easier on the verifier to multiply with 128
challenges as opposed to 256 bits. Second, the 256-bit challenges were
not actually uniformly distributed in a nice way because the challenges
get modded by the 254-bit scalar field modulus. In this approach, the
challenges should be distributed more uniformly (over 128-bit numbers).

Optimization(AztecProtocol/barretenberg#741) -
I could split every hash into 2 challenges, so I use all 256 bits of a
hash (when we need it), instead of throwing away 128 bits every time.

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [x] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [x] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).

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

@@ -71,32 +71,46 @@ template <typename FF> class BaseTranscript {
   private:
     static constexpr size_t MIN_BYTES_PER_CHALLENGE = 128 / 8; // 128 bit challenges
 
-    size_t round_number = 0;
+    size_t round_number = 0;        // current round for manifest
+    bool is_first_challenge = true; // indicates if this is the first challenge this transcript is generating
     std::array<uint8_t, HASH_OUTPUT_SIZE> previous_challenge_buffer{}; // default-initialized to zeros
     std::vector<uint8_t> current_round_data;
 
     // "Manifest" object that records a summary of the transcript interactions
     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());
+        // Prevent challenge generation if this is the first challenge we're generating,
+        // AND nothing was sent by the prover.
+        if (is_first_challenge) {
+            ASSERT(!current_round_data.empty());
+        }
 
-        // concatenate the hash of the previous round (if not the first round) with the current round data.
+        // 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 (!is_first_challenge) {
+            // if not the first challenge, we can use the previous_challenge_buffer
             full_buffer.insert(full_buffer.end(), previous_challenge_buffer.begin(), previous_challenge_buffer.end());
+        } else {
+            // Update is_first_challenge for the future
+            is_first_challenge = false;
+        }
+        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
         }
-        full_buffer.insert(full_buffer.end(), current_round_data.begin(), current_round_data.end());
 
         // 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 +123,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 +146,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 +163,25 @@ 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
+            // Note: because of how read() from buffers to fields works (in field_declarations.hpp),
+            // we use the later half of the buffer
+            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

@@ -139,8 +139,6 @@ TEST_F(UltraTranscriptTests, ProverManifestConsistency)
     auto manifest_expected = construct_ultra_honk_manifest(instance->proving_key->circuit_size);
     auto prover_manifest = prover.transcript.get_manifest();
     // Note: a manifest can be printed using manifest.print()
-    prover_manifest.print();
-    manifest_expected.print();
     for (size_t round = 0; round < manifest_expected.size(); ++round) {
         ASSERT_EQ(prover_manifest[round], manifest_expected[round]) << "Prover manifest discrepency in round " << round;
     }
@@ -171,9 +169,6 @@ TEST_F(UltraTranscriptTests, VerifierManifestConsistency)
     auto verifier = composer.create_verifier(instance);
     verifier.verify_proof(proof);
 
-    prover.transcript.print();
-    verifier.transcript.print();
-
     // Check consistency between the manifests generated by the prover and verifier
     auto prover_manifest = prover.transcript.get_manifest();
     auto verifier_manifest = verifier.transcript.get_manifest();
@@ -185,6 +180,30 @@ 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");
+    // check they are not 0
+    for (size_t i = 0; i < challenges.size(); ++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");
+    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;
@@ -216,9 +235,6 @@ TEST_F(UltraTranscriptTests, FoldingManifestTest)
     auto prover_res = prover.fold_instances();
     verifier.fold_public_parameters(prover_res.folding_data);
 
-    prover.transcript.print();
-    verifier.transcript.print();
-
     // Check consistency between the manifests generated by the prover and verifier
     auto prover_manifest = prover.transcript.get_manifest();
     auto verifier_manifest = verifier.transcript.get_manifest();