diff --git a/barretenberg/cpp/scripts/audit/audit_scopes/pedersen_hash_audit_scope.md b/barretenberg/cpp/scripts/audit/audit_scopes/pedersen_hash_audit_scope.md index 36e745bc2133..4669bbaf23e2 100644 --- a/barretenberg/cpp/scripts/audit/audit_scopes/pedersen_hash_audit_scope.md +++ b/barretenberg/cpp/scripts/audit/audit_scopes/pedersen_hash_audit_scope.md @@ -1,14 +1,16 @@ # Pedersen Hash Audit Scope Repository: https://github.com/AztecProtocol/aztec-packages -Commit hash: 4a956ceb179c2fe855e4f1fd78f2594e7fc3f5ea +Commit hash: ### Files to audit #### Native implementation 1. ```barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.hpp``` 2. ```barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.cpp``` -3. ```barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.hpp``` +3. ```barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.hpp``` +4. ```barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.cpp``` +5. ```barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.hpp``` ### Summary of the module @@ -34,6 +36,7 @@ Key features: #### Tests - ```barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp``` +- ```barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.test.cpp``` ### Documentation diff --git a/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.hpp b/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.hpp index b93f450651ab..288167cd801c 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.hpp @@ -62,12 +62,24 @@ template class generator_data { static constexpr std::span precomputed_generators = get_precomputed_generators(); + /** + * @brief Return a view of `num_generators` generators for a given `domain_separator`, starting at generator index + * `generator_offset`. + * + * @param num_generators number of generators to return + * @param generator_offset starting index of the generators to return + * @param domain_separator domain separator string + * @return GeneratorView + */ [[nodiscard]] inline GeneratorView get(const size_t num_generators, const size_t generator_offset = 0, const std::string_view domain_separator = DEFAULT_DOMAIN_SEPARATOR) const { const bool is_default_domain = domain_separator == DEFAULT_DOMAIN_SEPARATOR; - if (is_default_domain && (num_generators + generator_offset) < DEFAULT_NUM_GENERATORS) { + + // Case 1: we want default generators (is_default_domain), and the requested slice is contained within the + // precomputed array (num_generators + generator_offset <= DEFAULT_NUM_GENERATORS) + if (is_default_domain && (num_generators + generator_offset <= DEFAULT_NUM_GENERATORS)) { return GeneratorView{ precomputed_generators.data() + generator_offset, num_generators }; } @@ -84,7 +96,7 @@ template class generator_data { initialized_precomputed_generators = true; } - // if the generator map does not contain our desired generators, add entry into map + // Case 3: the requested domain is not contained in the generator map---add entry into map. if (!map.contains(std::string(domain_separator))) { map.insert({ std::string(domain_separator), @@ -93,8 +105,8 @@ template class generator_data { } GeneratorList& generators = map.at(std::string(domain_separator)); - - // If the current GeneratorList does not contain enough generators, extend it + // Case 4: the requested domain exists in the generator map but does not contain enough points to satisfy the + // request---extend the generator list. if (num_generators + generator_offset > generators.size()) { const size_t num_extra_generators = num_generators + generator_offset - generators.size(); GeneratorList extended_generators = @@ -121,7 +133,7 @@ template class generator_data { // the external functionality of `generator_data`. // i.e. `generator_data.get` will return the same output regardless of the internal state of `generator_data`. - // bool that describes whether we've copied the precomputed enerators into `generator_map`. This cannot be done at + // bool that describes whether we've copied the precomputed generators into `generator_map`. This cannot be done at // compile-time because std::map is a dynamically sized object. mutable bool initialized_precomputed_generators = false; diff --git a/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.test.cpp index ae5dbe2fa4ce..387bab42d045 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/generators/generator_data.test.cpp @@ -40,6 +40,65 @@ TEST(GeneratorContext, DeriveDefaultGenerators) for (size_t i = 0; i < default_generators.size(); ++i) { EXPECT_EQ(default_generators[i], expected_default_generators[i]); } + + // Verifies that the hard-coded precomputed generators match the derivation + EXPECT_TRUE((bb::check_precomputed_generators::DEFAULT_NUM_GENERATORS>())); +} + +// Tests generator_data::get() for different domain separators, num_generators, and offsets. +TEST(GeneratorContext, GeneratorDataGetVariousCases) +{ + generator_data generator_d; + + constexpr size_t default_num_generators = generator_data::DEFAULT_NUM_GENERATORS; // 8 + constexpr std::string_view default_domain_separator = + generator_data::DEFAULT_DOMAIN_SEPARATOR; // "DEFAULT_DOMAIN_SEPARATOR" + constexpr std::string_view domain_separator = "TEST_DOMAIN_SEPARATOR"; // custom non-default domain + + // Choose (n, offset) pairs that hit reach various branches in get(): (1) n+offset < 8, (2) n+offset == 8, + // (3)n+offset > 8 + struct Case { + size_t num_generators; + size_t offset; + }; + constexpr std::array cases{ { { 5, 0 }, { 6, 2 }, { 9, 1 } } }; + + // Test both default and non-default domain separators + constexpr std::array domains{ { default_domain_separator, domain_separator } }; + + for (auto domain : domains) { + for (auto test_case : cases) { + const size_t num_generators = test_case.num_generators; + const size_t offset = test_case.offset; + + // Compute generators using get() + auto generators = generator_d.get(num_generators, offset, domain); + ASSERT_EQ(generators.size(), num_generators); + + auto derived_generators = + grumpkin::g1::derive_generators(domain, num_generators + offset, /*starting_index=*/0); + ASSERT_EQ(derived_generators.size(), num_generators + offset); + + for (size_t i = 0; i < num_generators; ++i) { + EXPECT_EQ(generators[i], derived_generators[offset + i]); + } + + const bool is_default_domain = (domain == default_domain_separator); + + // For default-domain and num_generators + offset <= default_num_generators, the values must match + // precomputed_generators. + if (is_default_domain && (num_generators + offset <= default_num_generators)) { + auto default_generators = generator_data::precomputed_generators; + ASSERT_GE(default_generators.size(), num_generators + offset); + + for (size_t i = 0; i < num_generators; ++i) { + EXPECT_EQ(generators[i], default_generators[offset + i]); + } + } + } + } } } // namespace bb::crypto diff --git a/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.hpp b/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.hpp index 517ede529d9a..88b56d70866f 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.hpp @@ -1,5 +1,3 @@ -// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard - #pragma once #include "../generators/generator_data.hpp" #include "barretenberg/ecc/curves/bn254/bn254.hpp" diff --git a/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/README.md b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/README.md new file mode 100644 index 000000000000..9ac0d9c3a8a4 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/README.md @@ -0,0 +1,35 @@ +# Pedersen Hash and Commitment + +This module implements the Pedersen hash (`crypto/pedersen_hash`) and commitment (`crypto/pedersen_commitment`) over the Grumpkin curve. + +## Pederson Commitment +For a vector of field elements $x = [x_0, \dots, x_{n-1}]$, the commitment is $Commit(x) = \sum_{i=0}^{n-1} x_i \cdot G_i$, where $G = [G_0, G_1, \dots]$ are generators derived via `generator_data`. + +## Pederson Hash +For a vector of field elements $v = [v_0, \ldots, v_{n-1}]$, the hash is defined as the x-coordinate of $Hash(v) = n ยท H_{len} + Commit(v)$, where $H_{len}$ is a length generator derived with domain separator `"pedersen_hash_length"`. + +The length term avoids trivial collisions arising from the `x(P) = x(-P)` symmetry and to bind the hash to the input length. + +## API +### Hash +- `hash(const std::vector& inputs, const GeneratorContext context)` + - Hashes a vector of field elements using generators from `context`. + +- `hash_buffer(const std::vector& input, const GeneratorContext context)` + - Converts an arbitrary byte buffer into field elements by splitting into 31-byte chunks, followed by hashing. + - For >2 chunks it hashes iteratively: `r = hash(e0,e1); r = hash(r,e2); ...`. + +### Commitment +- `commit_native(const std::vector& inputs, GeneratorContext context)` + - Computes $Commit(inputs)$ using generators from `context`. + +## Tests +### Hash tests +- `DeriveLengthGenerator`: verifies that the `"pedersen_hash_length"` generator matches a known point. +- `Hash`: regression test for `hash({1,1})`. +- `HashWithIndex`: regression test for `hash({1,1}, offset=5)`. +- `Hash32Bytes`: checks iterative hashing performed within `hash_buffer` on a 32=31+1 byte input. + +### Commitment tests +- `Commitment`: regression test for `commit_native({1,1})`. +- `CommitmentWithZero`: regression test for `commit_native({0,1})`. diff --git a/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp index e920357626f0..984d6fcd5079 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp @@ -8,6 +8,7 @@ namespace bb::crypto { using bb::fr; +// Verifies the domain-seperated "pedersen_hash_length" generator matches the expected TEST(Pedersen, DeriveLengthGenerator) { auto generator = pedersen_hash::length_generator; @@ -18,6 +19,7 @@ TEST(Pedersen, DeriveLengthGenerator) fr(uint256_t("0x2ecd88d15967bc53b885912e0d16866154acb6aac2d3f85e27ca7eefb2c19083")))); } +// Verifies that hashing {1, 1} produces the expected result TEST(Pedersen, Hash) { auto x = pedersen_hash::Fq::one(); @@ -25,6 +27,7 @@ TEST(Pedersen, Hash) EXPECT_EQ(r, fr(uint256_t("07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b"))); } +// Verifies that hashing {1, 1} with a generator-offset/context variant produces the expected result TEST(Pedersen, HashWithIndex) { auto x = pedersen_hash::Fq::one(); @@ -32,4 +35,35 @@ TEST(Pedersen, HashWithIndex) EXPECT_EQ(r, fr(uint256_t("1c446df60816b897cda124524e6b03f36df0cec333fad87617aab70d7861daa6"))); } -} // namespace bb::crypto \ No newline at end of file +// Verifies that hashing a 32-byte buffer is equivalent to hashing two field elements via the intended chaining +TEST(Pedersen, Hash32Bytes) +{ + using Fq = pedersen_hash::Fq; + + std::vector buf(32); + for (size_t i = 0; i < buf.size(); ++i) { + buf[i] = static_cast(0xA0 + i); + } + + // First 31-byte chunk + uint256_t acc0(0); + for (size_t i = 0; i < 31; ++i) { + acc0 = (acc0 << uint256_t(8)); + acc0 += uint256_t(buf[i]); + } + Fq element0(acc0); + + // Last 1-byte chunk + uint256_t acc1(0); + acc1 = (acc1 << uint256_t(8)); + acc1 += uint256_t(buf[31]); + Fq element1(acc1); + + // For exactly 2 elements, hash_buffer should equal hash({element0, element1}) + auto expected = pedersen_hash::hash({ element0, element1 }); + auto got = pedersen_hash::hash_buffer(buf); + + EXPECT_EQ(got, expected); +} + +} // namespace bb::crypto