From f071f1b067a32f779e1b33845d5ac1ab8770ff38 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Mon, 2 Oct 2023 19:15:51 +0000 Subject: [PATCH 1/8] updated challenge generation to not be powers of each other --- .../honk/transcript/transcript.hpp | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index 4b99ce00b56..3edd921a4ae 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp @@ -85,17 +85,17 @@ template class BaseTranscript { */ [[nodiscard]] std::array get_next_challenge_buffer() const { + // Empty buffer to compare against + std::array empty_challenge_buffer{}; // Prevent challenge generation if nothing was sent by the prover. - ASSERT(!current_round_data.empty()); + ASSERT(!(current_round_data.empty() && previous_challenge_buffer == empty_challenge_buffer)); // concatenate the hash of the previous round (if not the first round) 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) { - full_buffer.insert(full_buffer.end(), previous_challenge_buffer.begin(), previous_challenge_buffer.end()); - } + 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()); // Pre-hash the full buffer to minimize the amount of data passed to the cryptographic hash function. @@ -145,26 +145,22 @@ 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 + current_round_data.clear(); // clear round data after generating challenge + std::array field_element_buffer{}; + std::copy_n(next_challenge_buffer.begin(), HASH_OUTPUT_SIZE, field_element_buffer.begin()); + challenges[i] = from_buffer(field_element_buffer); + previous_challenge_buffer = next_challenge_buffer; // update previous challenge buffer } // Prepare for next round. ++round_number; - current_round_data.clear(); - previous_challenge_buffer = next_challenge_buffer; return challenges; } From 0136da942a9dce2b28320922bb0a3c955ed82b79 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Wed, 4 Oct 2023 16:42:33 +0000 Subject: [PATCH 2/8] fixes to address review: updated comments and minor refactoring --- .../honk/transcript/transcript.hpp | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index 3edd921a4ae..c9220f9b84f 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp @@ -79,24 +79,34 @@ 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() { // Empty buffer to compare against std::array empty_challenge_buffer{}; - // Prevent challenge generation if nothing was sent by the prover. - ASSERT(!(current_round_data.empty() && previous_challenge_buffer == 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 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; - 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 (!no_previous_challenge) { + full_buffer.insert(full_buffer.end(), previous_challenge_buffer.begin(), previous_challenge_buffer.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 +119,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; }; @@ -152,11 +163,9 @@ template class BaseTranscript { // 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 - current_round_data.clear(); // clear round data after generating challenge std::array field_element_buffer{}; std::copy_n(next_challenge_buffer.begin(), HASH_OUTPUT_SIZE, field_element_buffer.begin()); challenges[i] = from_buffer(field_element_buffer); - previous_challenge_buffer = next_challenge_buffer; // update previous challenge buffer } // Prepare for next round. From 4dde4a35c426f381f9c8b1658a57189a34415a0b Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Wed, 4 Oct 2023 17:01:12 +0000 Subject: [PATCH 3/8] small comment updates --- .../src/barretenberg/honk/transcript/transcript.hpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index c9220f9b84f..6379f09d90e 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp @@ -83,7 +83,8 @@ template class BaseTranscript { * @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. + * 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() @@ -142,8 +143,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. From c1bd45d480ed99f0bcf648c79bb05f9f5cfa67d2 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 5 Oct 2023 19:10:02 +0000 Subject: [PATCH 4/8] added challenge generation test --- .../honk/transcript/transcript.test.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp index 8d9f55966da..6880cb71944 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp @@ -185,6 +185,19 @@ TEST_F(UltraTranscriptTests, VerifierManifestConsistency) } } +TEST_F(UltraTranscriptTests, ChallengeGenerationTest) +{ + auto transcript = ProverTranscript::init_empty(); + auto challenges = transcript.get_challenges("a", "b", "c", "d", "e", "f"); + for (size_t i = 0; i < challenges.size(); ++i) { + info("Challenge ", i, ": ", challenges[i]); + } + // check that they are not all 0 + for (size_t i = 0; i < challenges.size(); ++i) { + ASSERT_NE(challenges[i], 0) << "Challenge " << i << " is 0"; + } +} + TEST_F(UltraTranscriptTests, FoldingManifestTest) { using Flavor = flavor::Ultra; From 6eaa1a3395fb12001ca028841d756eed59fb7d73 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 5 Oct 2023 19:37:14 +0000 Subject: [PATCH 5/8] make challenges 128 bits --- .../cpp/src/barretenberg/honk/transcript/transcript.hpp | 2 ++ .../cpp/src/barretenberg/honk/transcript/transcript.test.cpp | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index 6379f09d90e..b0aa4c90b02 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp @@ -169,6 +169,8 @@ template class BaseTranscript { auto next_challenge_buffer = get_next_challenge_buffer(); // get next challenge buffer std::array field_element_buffer{}; std::copy_n(next_challenge_buffer.begin(), HASH_OUTPUT_SIZE, field_element_buffer.begin()); + // set upper half to 0 to make challenge 128 bits + std::fill_n(field_element_buffer.begin(), HASH_OUTPUT_SIZE / 2, 0); challenges[i] = from_buffer(field_element_buffer); } diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp index 6880cb71944..0dbb7732842 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp @@ -189,11 +189,9 @@ TEST_F(UltraTranscriptTests, ChallengeGenerationTest) { auto transcript = ProverTranscript::init_empty(); 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]); - } - // check that they are not all 0 - for (size_t i = 0; i < challenges.size(); ++i) { ASSERT_NE(challenges[i], 0) << "Challenge " << i << " is 0"; } } From 2e559217e0bba4f463ca390ab9bded1583d04376 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 5 Oct 2023 20:00:31 +0000 Subject: [PATCH 6/8] minor update to test, challenge instantiation --- .../barretenberg/honk/transcript/transcript.hpp | 7 ++++--- .../honk/transcript/transcript.test.cpp | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index b0aa4c90b02..feaf6c66b15 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp @@ -168,9 +168,10 @@ template class BaseTranscript { 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{}; - std::copy_n(next_challenge_buffer.begin(), HASH_OUTPUT_SIZE, field_element_buffer.begin()); - // set upper half to 0 to make challenge 128 bits - std::fill_n(field_element_buffer.begin(), HASH_OUTPUT_SIZE / 2, 0); + // 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(field_element_buffer); } diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp index 0dbb7732842..38bcb93aeb3 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp @@ -185,15 +185,32 @@ 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"); // 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) From 3455f3c82e3998857d8e350a10b8f6345f0a9e5a Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 5 Oct 2023 21:43:07 +0000 Subject: [PATCH 7/8] fixed formatting and removed infos --- .../barretenberg/honk/transcript/transcript.hpp | 9 ++++----- .../honk/transcript/transcript.test.cpp | 14 +------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index feaf6c66b15..719ae1efae5 100644 --- a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp @@ -81,10 +81,9 @@ template class BaseTranscript { /** * @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. + * 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() @@ -146,7 +145,7 @@ template class BaseTranscript { * 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 + * 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 diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.test.cpp index 38bcb93aeb3..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(); @@ -196,18 +191,14 @@ TEST_F(UltraTranscriptTests, ChallengeGenerationTest) auto transcript = ProverTranscript::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 + // 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"; @@ -244,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(); From 123103953f65673a5dd4ee3ce4ed0c5e094b4176 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 5 Oct 2023 21:56:42 +0000 Subject: [PATCH 8/8] updated checking on first challenge, extra comment --- .../honk/transcript/transcript.hpp | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/honk/transcript/transcript.hpp index 719ae1efae5..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; @@ -88,20 +89,23 @@ template class BaseTranscript { */ [[nodiscard]] std::array get_next_challenge_buffer() { - // Empty buffer to compare against - std::array 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)); + // 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 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 (!no_previous_challenge) { + 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()); @@ -168,6 +172,8 @@ template class BaseTranscript { 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);