From 68c1fab51e3a339032b719ce966ed34787f33dab Mon Sep 17 00:00:00 2001 From: Lucas Xia Date: Thu, 5 Oct 2023 18:21:39 -0400 Subject: [PATCH] fix: challenge generation update (#2628) This PR updates challenge generation to not be powers of each other, fixing https://github.com/AztecProtocol/barretenberg/issues/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(https://github.com/AztecProtocol/barretenberg/issues/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). --- .../honk/transcript/transcript.hpp | 65 ++++++++++++------- .../honk/transcript/transcript.test.cpp | 32 ++++++--- 2 files changed, 65 insertions(+), 32 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index 4b99ce00b56..042a0c561b5 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp @@ -71,7 +71,8 @@ template 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 previous_challenge_buffer{}; // default-initialized to zeros std::vector current_round_data; @@ -79,24 +80,37 @@ template 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 */ - [[nodiscard]] std::array get_next_challenge_buffer() const + [[nodiscard]] std::array 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 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 class BaseTranscript { std::array 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 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 challenges for this round. @@ -145,26 +163,25 @@ template 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 challenges{}; - std::array field_element_buffer{}; - std::copy_n(next_challenge_buffer.begin(), HASH_OUTPUT_SIZE, field_element_buffer.begin()); - - challenges[0] = from_buffer(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 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(field_element_buffer); } // Prepare for next round. ++round_number; - current_round_data.clear(); - previous_challenge_buffer = next_challenge_buffer; return challenges; } diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp index 8d9f55966da..df3ca8526dd 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp +++ b/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::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();