Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: consistent pedersen hash (work in progress) #1945

Merged
merged 55 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
2acca41
added cycle_group class
zac-williamson Aug 15, 2023
4f885f6
fixed bugs in cycle_group add/sub/dbl
zac-williamson Aug 16, 2023
fbd9b40
variable-base scalar multiplication passes tests
zac-williamson Aug 17, 2023
55f7019
cycle_group::variable_batch_mul now supports input points that are at…
zac-williamson Aug 17, 2023
e339ba0
added an elliptic curve point doubling gate to the UltraPlonk arithme…
zac-williamson Aug 17, 2023
ce45f32
hash to curve
zac-williamson Aug 21, 2023
f5a9a5f
wip
zac-williamson Aug 25, 2023
604ad3f
fixed linting errors in proof_system/plookup_tables
zac-williamson Aug 26, 2023
c09483e
added refactored pedersen hash methods + stdlib::pedersen_hash (needs…
zac-williamson Aug 29, 2023
36624f3
fixed, tidy up, comments
zac-williamson Sep 2, 2023
0608a65
wip
zac-williamson Sep 10, 2023
dcec4f0
Merge remote-tracking branch 'origin/master' into zw/stdlib-cycle-gro…
charlielye Sep 15, 2023
afad6f0
completed merge of master
zac-williamson Sep 15, 2023
4283697
Merge branch 'master' into zw/stdlib-cycle-group-msm
zac-williamson Sep 15, 2023
43086fa
revert formatting
zac-williamson Sep 15, 2023
918410c
removed extra ecc gate methods from standard/turbo circuit builder
zac-williamson Sep 15, 2023
0db1732
wip
zac-williamson Sep 15, 2023
98e5261
PR changes
zac-williamson Sep 15, 2023
af2a6ac
fixed ecc_dbl gates incorrectly fusing into ecc_add gates
zac-williamson Sep 15, 2023
4426370
wip
zac-williamson Sep 15, 2023
fd0de30
compiler fixes
zac-williamson Sep 20, 2023
a29ebb3
compiler fox
zac-williamson Sep 20, 2023
79b17e1
compiler fox
zac-williamson Sep 20, 2023
a0c9f92
compiler fix
zac-williamson Sep 20, 2023
5e7a4d5
compiler fix
zac-williamson Sep 20, 2023
8b58e39
reverted schnorr
zac-williamson Sep 21, 2023
87ff132
compiler fix
zac-williamson Sep 21, 2023
9236506
revert pedersen c_bind
zac-williamson Sep 21, 2023
c899d90
revert crypto/schnorr
zac-williamson Sep 21, 2023
a5254f1
Merge branch 'master' into zw/stdlib-cycle-group-msm
zac-williamson Sep 21, 2023
6c8adcc
bugfix
zac-williamson Sep 22, 2023
0dfc607
Merge branch 'master' into zw/stdlib-cycle-group-msm
zac-williamson Sep 22, 2023
f02c8ae
Yeet.
zac-williamson Sep 28, 2023
6d7553e
remove oof
zac-williamson Sep 28, 2023
e96faf1
comment fixes
zac-williamson Sep 28, 2023
5659240
comments
zac-williamson Sep 28, 2023
730e7ee
fix
zac-williamson Sep 28, 2023
0c7e955
fix
zac-williamson Sep 28, 2023
f782338
fix
zac-williamson Sep 28, 2023
3db1576
comments
zac-williamson Sep 28, 2023
664c869
fix
zac-williamson Sep 28, 2023
be67530
fix
zac-williamson Sep 28, 2023
9563341
fix
zac-williamson Sep 28, 2023
c0ce229
fix
zac-williamson Sep 28, 2023
e672785
fix
zac-williamson Sep 28, 2023
246bf4f
more fix
zac-williamson Sep 28, 2023
d7c120e
names are hard
zac-williamson Sep 28, 2023
c146851
fix
zac-williamson Sep 28, 2023
de6d232
typo
zac-williamson Sep 28, 2023
a04b4b4
test fix
zac-williamson Sep 29, 2023
031c108
Merge branch 'master' into zw/stdlib-cycle-group-msm
zac-williamson Sep 29, 2023
b79cac4
fix
zac-williamson Sep 29, 2023
20bf104
merge fix
zac-williamson Sep 29, 2023
760aba7
comments
zac-williamson Sep 29, 2023
f1b3271
bugfix
zac-williamson Sep 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ relations.bench.cpp
# Required libraries for benchmark suites
set(LINKED_LIBRARIES
polynomials
proof_system
benchmark::benchmark
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#pragma once

// TODO(@zac-williamson #2341 delete this file once we migrate to new pedersen hash standard)

#include "./generator_data.hpp"
#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "./generator_data.hpp"

// TODO(@zac-williamson #2341 delete this file once we migrate to new pedersen hash standard)

namespace crypto {
namespace generators {
namespace {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#pragma once

// TODO(@zac-williamson #2341 delete this file once we migrate to new pedersen hash standard)
#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
#include <array>
#include <tuple>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-williamson #2341 delete this file once we migrate to new pedersen hash standard)

#include "./generator_data.hpp"
#include "./fixed_base_scalar_mul.hpp"
#include "barretenberg/common/streams.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file and rename c_bind_new to c_bind once we have migrated to new hash standard

#include "c_bind.hpp"
#include "barretenberg/common/mem.hpp"
#include "barretenberg/common/serialize.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file and rename c_bind_new to c_bind once we have migrated to new hash standard

#pragma once
#include "barretenberg/common/mem.hpp"
#include "barretenberg/common/serialize.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "./pedersen.hpp"
#include "./convert_buffer_to_field.hpp"
#include "barretenberg/common/throw_or_abort.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#pragma once
#include "../generators/fixed_base_scalar_mul.hpp"
#include "../generators/generator_data.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "./pedersen_lookup.hpp"
#include "../pedersen_hash/pedersen_lookup.hpp"
#include "./convert_buffer_to_field.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#pragma once

// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"

namespace crypto {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "barretenberg/numeric/bitop/get_msb.hpp"
#include "barretenberg/numeric/random/engine.hpp"
#include <gtest/gtest.h>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// TODO(@zac-wiliamson #2341 rename to pedersen.cpp once we migrate to new hash standard)

#include "./pedersen_refactor.hpp"
#include "./convert_buffer_to_field.hpp"
#include "barretenberg/common/serialize.hpp"
#include "barretenberg/common/throw_or_abort.hpp"
#include <iostream>
#ifndef NO_OMP_MULTITHREADING
#include <omp.h>
#endif

namespace crypto {

/**
* @brief Given a vector of fields, generate a pedersen commitment using the indexed generators.
*
* @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating
* grumpkin commitments to field elements inside a BN254 SNARK circuit.
*
* @note Fq is the *coordinate field* of Curve. Curve itself is a SNARK-friendly curve,
* i.e. Fq represents the native field type of the SNARK circuit.
* @param inputs
* @param hash_index
* @param generator_context
* @return Curve::AffineElement
*/
template <typename Curve>
typename Curve::AffineElement pedersen_commitment_refactor<Curve>::commit_native(
const std::vector<Fq>& inputs, const size_t hash_index, const generator_data<Curve>* const generator_context)
{
const auto generators = generator_context->conditional_extend(inputs.size() + hash_index);
Element result = Group::point_at_infinity;

// `Curve::Fq` represents the field that `Curve` is defined over (i.e. x/y coordinate field) and `Curve::Fr` is the
// field whose modulus = the group order of `Curve`.
// The `Curve` we're working over here is a generic SNARK-friendly curve. i.e. the SNARK circuit is defined over a
// field equivalent to `Curve::Fq`. This adds complexity when we wish to commit to SNARK circuit field elements, as
// these are members of `Fq` and *not* `Fr`. We cast to `uint256_t` in order to convert an element of `Fq` into an
// `Fr` element, which is the required type when performing scalar multiplications.
static_assert(Fr::modulus > Fq::modulus,
"pedersen_commitment::commit_native Curve subgroup field is smaller than coordinate field. Cannot "
"perform injective conversion");
for (size_t i = 0; i < inputs.size(); ++i) {
Fr scalar_multiplier(static_cast<uint256_t>(inputs[i]));
result += Element(generators.get(i, hash_index)) * scalar_multiplier;
}
return result;
}

template class pedersen_commitment_refactor<curve::Grumpkin>;
} // namespace crypto
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
#pragma once

// TODO(@zac-wiliamson #2341 rename to pedersen.hpp once we migrate to new hash standard)

#include "../generators/fixed_base_scalar_mul.hpp"
#include "../generators/generator_data.hpp"
#include "barretenberg/ecc/curves/bn254/bn254.hpp"
#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
#include <array>

namespace crypto {

/**
* @brief Contains a vector of precomputed generator points.
* Generators are defined via a domain separator.
* Number of generators in generator_data is fixed for a given object instance.
*
* @details generator_data is used to precompute short lists of commonly used generators,
* (e.g. static inline const default_generators = generator_data()).
* If an algorithm requires more than `_size_ generators,
* the `conditional_extend` method can be called to return a new `generator_data` object.
* N.B. we explicitly do not support mutating an existing `generator_data` object to increase the size of
* its `std::vector<affine_element> generators` member variable.
* This is because this class is intended to be used as a `static` member of other classes to provide lists
* of precomputed generators. Mutating static member variables is *not* thread safe!
*/
template <typename Curve> class generator_data {
public:
using Group = typename Curve::Group;
using AffineElement = typename Curve::AffineElement;
static inline constexpr size_t DEFAULT_NUM_GENERATORS = 32;
static inline const std::string DEFAULT_DOMAIN_SEPARATOR = "default_domain_separator";
inline generator_data(const size_t num_generators = DEFAULT_NUM_GENERATORS,
const std::string& domain_separator = DEFAULT_DOMAIN_SEPARATOR)
: _domain_separator(domain_separator)
, _domain_separator_bytes(domain_separator.begin(), domain_separator.end())
, _size(num_generators){};

[[nodiscard]] inline std::string domain_separator() const { return _domain_separator; }
[[nodiscard]] inline size_t size() const { return _size; }
[[nodiscard]] inline AffineElement get(const size_t index, const size_t offset = 0) const
{
ASSERT(index + offset <= _size);
return generators[index + offset];
}

/**
* @brief If more generators than `_size` are required, this method will return a new `generator_data` object
* with the required generators.
*
* @note Question: is this a good pattern to support? Ideally downstream code would ensure their
* `generator_data` object is sufficiently large to cover potential needs.
* But if we did not support this pattern, it would make downstream code more complex as each method that
* uses `generator_data` would have to perform this accounting logic.
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
*
* @param target_num_generators
* @return generator_data
*/
[[nodiscard]] inline generator_data conditional_extend(const size_t target_num_generators) const
{
if (target_num_generators <= _size) {
return *this;
}
return { target_num_generators, _domain_separator };
}

private:
std::string _domain_separator;
std::vector<uint8_t> _domain_separator_bytes;
size_t _size;
// ordering of static variable initialization is undefined, so we make `default_generators` private
// and only accessible via `get_default_generators()`, which ensures var will be initialized at the cost of some
// small runtime checks
inline static const generator_data default_generators =
generator_data(generator_data::DEFAULT_NUM_GENERATORS, generator_data::DEFAULT_DOMAIN_SEPARATOR);

public:
inline static const generator_data* get_default_generators() { return &default_generators; }
const std::vector<AffineElement> generators = (Group::derive_generators_secure(_domain_separator_bytes, _size));
};

template class generator_data<curve::Grumpkin>;

/**
* @brief Performs pedersen commitments!
*
* To commit to a size-n list of field elements `x`, a commitment is defined as:
*
* Commit(x) = x[0].g[0] + x[1].g[1] + ... + x[n-1].g[n-1]
*
* Where `g` is a list of generator points defined by `generator_data`
*
*/
template <typename Curve> class pedersen_commitment_refactor {
public:
using AffineElement = typename Curve::AffineElement;
using Element = typename Curve::Element;
using Fr = typename Curve::ScalarField;
using Fq = typename Curve::BaseField;
using Group = typename Curve::Group;

static AffineElement commit_native(
const std::vector<Fq>& inputs,
size_t hash_index = 0,
const generator_data<Curve>* generator_context = generator_data<Curve>::get_default_generators());
};

extern template class pedersen_commitment_refactor<curve::Grumpkin>;
} // namespace crypto
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file and rename c_bind_new to c_bind once we have migrated to new hash standard

#include "barretenberg/common/mem.hpp"
#include "barretenberg/common/serialize.hpp"
#include "barretenberg/common/streams.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#pragma once
// TODO(@zac-wiliamson #2341 delete this file and rename c_bind_new to c_bind once we have migrated to new hash standard

#include "barretenberg/common/wasm_export.hpp"
#include "barretenberg/ecc/curves/bn254/fr.hpp"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "./pedersen.hpp"
#include <iostream>
#ifndef NO_OMP_MULTITHREADING
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#pragma once

// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "../generators/fixed_base_scalar_mul.hpp"
#include "../generators/generator_data.hpp"
#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "./pedersen_lookup.hpp"

#include <mutex>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#pragma once
// TODO(@zac-wiliamson #2341 delete this file once we migrate to new hash standard

#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#include "./pedersen_refactor.hpp"
#include <iostream>
#ifndef NO_OMP_MULTITHREADING
#include <omp.h>
#endif

// TODO(@zac-wiliamson #2341 rename to pedersen.cpp once we migrate to new hash standard)

namespace crypto {

using namespace generators;

/**
* Given a vector of fields, generate a pedersen hash using the indexed generators.
*/

/**
* @brief Given a vector of fields, generate a pedersen hash using generators from `generator_context`.
*
* @details `hash_index` is used to access offset elements of `generator_context` if required.
* e.g. if one desires to compute
* `inputs[0] * [generators[hash_index]] + `inputs[1] * [generators[hash_index + 1]]` + ... etc
* Potentially useful to ensure multiple hashes with the same domain separator cannot collide.
*
* TODO(@suyash67) can we change downstream code so that `hash_index` is no longer required? Now we have a proper
* domain_separator parameter, we no longer need to specify different generator indices to ensure hashes cannot collide.
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
* @param inputs what are we hashing?
* @param hash_index Describes an offset into the list of generators, if required
* @param generator_context
* @return Fq (i.e. SNARK circuit scalar field, when hashing using a curve defined over the SNARK circuit scalar field)
*/
template <typename Curve>
typename Curve::BaseField pedersen_hash_refactor<Curve>::hash_multiple(const std::vector<Fq>& inputs,
const size_t hash_index,
const generator_data* const generator_context)
{
const auto generators = generator_context->conditional_extend(inputs.size() + hash_index);

Element result = get_length_generator() * Fr(inputs.size());

for (size_t i = 0; i < inputs.size(); ++i) {
result += generators.get(i, hash_index) * Fr(static_cast<uint256_t>(inputs[i]));
}
result = result.normalize();
return result.x;
Comment on lines +44 to +45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orthogonal to this PR so can ignore; This pattern seems rather footgunny; If I accidentally pull out the x co-ordinate without converting it to affine form by normalizing, I would essentially be getting the projective value for X.

I can open up a separate issue -> A fix would revolve around safe-guarding access to x

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that'd be a good idea.

}

template <typename Curve>
typename Curve::BaseField pedersen_hash_refactor<Curve>::hash(const std::vector<Fq>& inputs,
size_t hash_index,
const generator_data* const generator_context)
{
return hash_multiple(inputs, hash_index, generator_context);
}

template class pedersen_hash_refactor<curve::Grumpkin>;
} // namespace crypto
Loading