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: circuit simulator for Ultra and GoblinUltra verifiers #1195

Merged
merged 69 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
3497303
feat: barretenberg PR migrate tool
ludamad Jul 25, 2023
c4d7963
Simulator
codygunton Jul 26, 2023
42878d0
Reinstate asserts.
codygunton Jul 26, 2023
4ea80be
Give up and skip test.
codygunton Jul 26, 2023
5aa32cf
Manually pick non-B. changes.
codygunton Jul 26, 2023
0fcabd8
Add one high generator.
codygunton Jul 26, 2023
ca674c3
Merge branch 'master' into cg/simulate-spike-bb-subrepo
codygunton Jul 26, 2023
daa37eb
Reset files I didn't touch
codygunton Jul 26, 2023
d818dfd
fix: merge and ci workarounds
ludamad Jul 26, 2023
c0790fe
Merge branch 'master' into cg/simulate-spike-bb-subrepo
ludamad Jul 27, 2023
2cb9f9d
Merge branch 'master' into cg/simulate-spike-bb-subrepo
ludamad Jul 27, 2023
b707765
Compare Pedersen benchmarks.
codygunton Aug 7, 2023
378b899
blake3s
codygunton Aug 8, 2023
c42b000
Ecdsa / secp256k1
codygunton Aug 8, 2023
6a2c226
Biggroup batch_mul
codygunton Aug 8, 2023
65a3997
Speed up biggroup batch mul.
codygunton Aug 8, 2023
19ed7b5
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
codygunton Aug 8, 2023
dfe4af6
Changes missed in merge commit.
codygunton Aug 8, 2023
7c67671
Try to satisfy GCC
codygunton Aug 9, 2023
3f2862a
Tweak script output.
codygunton Aug 9, 2023
b963361
Speed up ECDSA simulation
codygunton Aug 9, 2023
6e0069b
Add missing brace
codygunton Aug 9, 2023
8744b27
Use derived context.
codygunton Aug 10, 2023
ec3346a
Set context so merkel test will run.
codygunton Aug 10, 2023
dfbf067
Fix script comments
codygunton Aug 15, 2023
1709038
TODO(https://github.com/AztecProtocol/barretenberg/issues/659)
codygunton Aug 15, 2023
05101f3
Cleanup and issue issues.
codygunton Aug 16, 2023
f4665e0
Remove unneeded function.
codygunton Aug 17, 2023
e81f015
Reinstate missing recursion check.
codygunton Aug 17, 2023
94d5afd
Clean up and add comments.
codygunton Aug 17, 2023
890c374
Intentionally do a bad merge.
ludamad Sep 6, 2023
5659262
fix e2e-escrow
ludamad Sep 6, 2023
39f6efe
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
ludamad0 Sep 6, 2023
9e2042f
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
ludamad0 Sep 6, 2023
586241e
chore: merge master into simulate feature branch (#1848)
ludamad Sep 14, 2023
fed8f7e
Merge
ludamad0 Sep 14, 2023
999451d
Rm bb
ludamad0 Sep 14, 2023
0a5f699
git mv bb
ludamad0 Sep 14, 2023
a168a6d
Rm bb/build-system
ludamad0 Sep 14, 2023
38882cf
Merge commit '404ec34d38e1a9c3fbe7a3cdb6e88c28f62f72e4' into cg/simul…
ludamad0 Sep 14, 2023
7c0f781
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
ludamad0 Sep 14, 2023
20dfb60
Fix merge markers
ludamad0 Sep 14, 2023
581fbb3
Format
ludamad0 Sep 14, 2023
15ed3a9
Reverts
ludamad0 Sep 14, 2023
f4a62bb
Merge branch 'master' into cg/simulate-spike-bb-subrepo
maramihali Apr 18, 2024
560f7bf
fine till here
maramihali Apr 25, 2024
05f9913
experiments with ultra verifier
maramihali Apr 26, 2024
75dd2dd
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
maramihali Apr 26, 2024
8df0c86
unify honk recursive verifiers
maramihali Apr 26, 2024
3132542
yay
maramihali Apr 29, 2024
2edbcf7
stuff
maramihali Apr 29, 2024
7462d38
add benchmark
maramihali Apr 29, 2024
60d7197
more cleanup
maramihali Apr 29, 2024
ebe0996
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
maramihali Apr 29, 2024
f7183b5
more cleanup
maramihali Apr 29, 2024
9bee66b
reenable check, more cleanup
maramihali Apr 29, 2024
1f4a260
cleanup benchmark
maramihali Apr 29, 2024
96eb8df
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
maramihali Apr 29, 2024
b429bef
fix gcc and test
maramihali Apr 29, 2024
80b06c6
more cleanup
maramihali Apr 29, 2024
6b5ae50
even more cleanup
maramihali Apr 29, 2024
3728048
delete some more unnecessary stuff
maramihali Apr 29, 2024
4a169b4
more cleanup
maramihali Apr 29, 2024
4d98946
revert some formatting
maramihali Apr 29, 2024
2d4355e
remove unnecessary fields in simulator
maramihali Apr 29, 2024
ca81f60
follup up after cody's comments
maramihali Apr 29, 2024
ff03998
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
maramihali Apr 29, 2024
424b670
Merge remote-tracking branch 'origin/master' into cg/simulate-spike-b…
maramihali Apr 30, 2024
cbda9ec
Merge branch 'master' into cg/simulate-spike-bb-subrepo
maramihali Apr 30, 2024
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
2 changes: 1 addition & 1 deletion barretenberg/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ Or install from a package manager, on Ubuntu:
sudo apt-get install libomp-dev
```

> Note: on a fresh Ubuntu Kinetic installation, installing OpenMP from source yields a `Could NOT find OpenMP_C (missing: OpenMP_omp_LIBRARY) (found version "5.0")` error when trying to build Barretenberg. Installing from apt worked fine.
> Note: on a fresh Ubuntu lunar installation, installing OpenMP from source yields a `Could NOT find OpenMP_C (missing: OpenMP_omp_LIBRARY) (found version "5.0")` error when trying to build Barretenberg. Installing from apt worked fine.
Copy link
Contributor

Choose a reason for hiding this comment

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

was this change intended?


### Getting started

Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/benchmark/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ add_subdirectory(ipa_bench)
add_subdirectory(client_ivc_bench)
add_subdirectory(pippenger_bench)
add_subdirectory(plonk_bench)
add_subdirectory(simulator_bench)
add_subdirectory(protogalaxy_bench)
add_subdirectory(protogalaxy_rounds_bench)
add_subdirectory(relations_bench)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
barretenberg_module(simulator_bench stdlib_honk_recursion stdlib_sha256 crypto_merkle_tree)
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
#include "barretenberg/goblin/goblin.hpp"
#include "barretenberg/goblin/mock_circuits.hpp"
#include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp"
#include <benchmark/benchmark.h>

using namespace benchmark;
using namespace bb;

namespace {
template <typename RecursiveFlavor> class SimulatorFixture : public benchmark::Fixture {

public:
using Flavor = typename RecursiveFlavor::NativeFlavor;
using ProverInstance = ProverInstance_<Flavor>;
using Builder = typename Flavor::CircuitBuilder;
using VerificationKey = typename Flavor::VerificationKey;
using CircuitSimulator = typename RecursiveFlavor::CircuitBuilder;
using SimulatingVerifier = stdlib::recursion::honk::UltraRecursiveVerifier_<RecursiveFlavor>;

struct VerifierInput {
HonkProof proof;
std::shared_ptr<VerificationKey> verification_key;
};

void SetUp([[maybe_unused]] const ::benchmark::State& state) override
{
bb::srs::init_crs_factory("../srs_db/ignition");
}

/**
* @brief Create a Honk proof (either Ultra or GoblinUltra) for a non-trivial circuit.
*
* @param large determines whether the circuit is 2^17 or 2^19
*/
static VerifierInput create_proof(bool large = false)
{

auto builder = construct_mock_function_circuit(large);
auto instance = std::make_shared<ProverInstance>(builder);
UltraProver_<Flavor> prover(instance);
auto ultra_proof = prover.construct_proof();
auto verification_key = std::make_shared<VerificationKey>(instance->proving_key);
return { ultra_proof, verification_key };
}

/**
* @brief Populate the builder with non-trivial operations that mock a circuit encountered in practice.
*
* @param large determines whether the circuit is 2^17 or 2^19
*/
static Builder construct_mock_function_circuit(bool large = false)
{
using InnerCurve = bb::stdlib::bn254<Builder>;
using fr_ct = InnerCurve::ScalarField;
using point_ct = InnerCurve::AffineElement;
using fr = typename InnerCurve::ScalarFieldNative;
using point = typename InnerCurve::GroupNative::affine_element;
Builder builder;

// Perform a batch mul which will add some arbitrary goblin-style ECC op gates if the circuit arithmetic is
// goblinisied otherwise it will add the conventional nonnative gates
size_t num_points = 5;
std::vector<point_ct> circuit_points;
std::vector<fr_ct> circuit_scalars;
for (size_t i = 0; i < num_points; ++i) {
circuit_points.push_back(point_ct::from_witness(&builder, point::random_element()));
circuit_scalars.push_back(fr_ct::from_witness(&builder, fr::random_element()));
}
point_ct::batch_mul(circuit_points, circuit_scalars);

// Determine number of times to execute the below operations that constitute the mock circuit logic. Note
// that the circuit size does not scale linearly with number of iterations due to e.g. amortization of lookup

const size_t NUM_ITERATIONS_LARGE = 12; // results in circuit size 2^19 (502238 gates)
const size_t NUM_ITERATIONS_MEDIUM = 3; // results in circuit size 2^17 (124843 gates)
const size_t NUM_ITERATIONS = large ? NUM_ITERATIONS_LARGE : NUM_ITERATIONS_MEDIUM;

stdlib::generate_sha256_test_circuit(builder, NUM_ITERATIONS); // min gates: ~39k
stdlib::generate_ecdsa_verification_test_circuit(builder, NUM_ITERATIONS); // min gates: ~41k
stdlib::generate_merkle_membership_test_circuit(builder, NUM_ITERATIONS); // min gates: ~29k

return builder;
}
};

BENCHMARK_TEMPLATE_F(SimulatorFixture, GoblinNative, bb::GoblinUltraRecursiveFlavor_<bb::CircuitSimulatorBN254>)
Copy link
Contributor

@maramihali maramihali Apr 29, 2024

Choose a reason for hiding this comment

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

This and UltraNative are basically the same code (ditto for the simulated ones) but I haven't managed to instantiate same benchmark functions with multiple templates, not even sure it's supported. Very much open to suggestions

(benchmark::State& state)
{
auto verifier_input = SimulatorFixture::create_proof();
for (auto _ : state) {
UltraVerifier_<Flavor> ultra_verifier{ verifier_input.verification_key };
ultra_verifier.verify_proof((verifier_input.proof));
}
}

BENCHMARK_TEMPLATE_F(SimulatorFixture, GoblinSimulated, bb::GoblinUltraRecursiveFlavor_<bb::CircuitSimulatorBN254>)
(benchmark::State& state)
{
auto verifier_input = SimulatorFixture::create_proof();
for (auto _ : state) {
CircuitSimulator simulator;
SimulatingVerifier ultra_verifier{ &simulator, verifier_input.verification_key };
ultra_verifier.verify_proof((verifier_input.proof));
}
}

BENCHMARK_TEMPLATE_F(SimulatorFixture, UltraNative, bb::UltraRecursiveFlavor_<bb::CircuitSimulatorBN254>)
(benchmark::State& state)
{
auto verifier_input = SimulatorFixture::create_proof();
for (auto _ : state) {
UltraVerifier_<typename SimulatorFixture::Flavor> ultra_verifier{ verifier_input.verification_key };
ultra_verifier.verify_proof((verifier_input.proof));
}
}

BENCHMARK_TEMPLATE_F(SimulatorFixture, UltraSimulated, bb::UltraRecursiveFlavor_<bb::CircuitSimulatorBN254>)
(benchmark::State& state)
{
auto verifier_input = SimulatorFixture::create_proof();
for (auto _ : state) {
CircuitSimulator simulator;
SimulatingVerifier ultra_verifier{ &simulator, verifier_input.verification_key };
ultra_verifier.verify_proof((verifier_input.proof));
}
}

BENCHMARK_REGISTER_F(SimulatorFixture, GoblinSimulated)->Unit(benchmark::kMillisecond);
BENCHMARK_REGISTER_F(SimulatorFixture, UltraSimulated)->Unit(benchmark::kMillisecond);
BENCHMARK_REGISTER_F(SimulatorFixture, GoblinNative)->Unit(benchmark::kMillisecond);
BENCHMARK_REGISTER_F(SimulatorFixture, UltraNative)->Unit(benchmark::kMillisecond);

} // namespace
BENCHMARK_MAIN();
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ concept IsCheckable = bb::IsAnyOf<T,
StandardCircuitBuilder_<bb::fr>,
StandardCircuitBuilder_<bb::fq>,
UltraCircuitBuilder,
GoblinUltraCircuitBuilder>;
GoblinUltraCircuitBuilder,
CircuitSimulatorBN254>;

/**
* @brief The unified interface for check circuit functionality implemented in the specialized CircuitChecker classes
Expand All @@ -28,6 +29,8 @@ class CircuitChecker {
return UltraCircuitChecker::check(builder);
} else if constexpr (IsStandardBuilder<Builder>) {
return StandardCircuitChecker::check(builder);
} else if constexpr (IsSimulator<Builder>) {
return SimulatorCircuitChecker::check(builder);
} else {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ ecdsa_signature ecdsa_construct_signature(const std::string& message, const ecds
template <typename Hash, typename Fq, typename Fr, typename G1>
typename G1::affine_element ecdsa_recover_public_key(const std::string& message, const ecdsa_signature& sig);

// TODO(https://github.com/AztecProtocol/barretenberg/issues/659)
template <typename Hash, typename Fq, typename Fr, typename G1>
bool ecdsa_verify_signature(const std::string& message,
const typename G1::affine_element& public_key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ void assert_check_subtree_membership(field_t<Builder> const& root,
std::string const& msg = "assert_check_subtree_membership")
{
auto exists = check_subtree_membership(root, hashes, value, index, at_height, is_updating_tree);
exists.context = root.context;
exists.assert_equal(true, msg);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,11 @@ void compute_wnaf_states(uint64_t* point_schedule,
}

parallel_for(num_threads, [&](size_t i) {
Fr T0;
// NOTE: to appease GCC array-bounds checks, we need an extra Fr at the end of 'T0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to the simulator? If so we should probably document why. Otherwise this should go in a separate PR because it feels pretty fundamental

Copy link
Contributor

Choose a reason for hiding this comment

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

i think it can be reverted

// This is why it is an Fr[2]. Otherwise, GCC really doesn't like us type-punning in endo_scalar_upper_limbs as
// it encompasses undefined memory (even though it doesn't use it).
Fr T0_buffer[2];
Fr& T0 = T0_buffer[0];
uint64_t* wnaf_table = &point_schedule[(2 * i) * num_initial_points_per_thread];
const Fr* thread_scalars = &scalars[i * num_initial_points_per_thread];
bool* skew_table = &input_skew_table[(2 * i) * num_initial_points_per_thread];
Expand Down
10 changes: 7 additions & 3 deletions barretenberg/cpp/src/barretenberg/flavor/flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,22 +343,26 @@ concept IsUltraFlavor = IsAnyOf<T, UltraFlavor, GoblinUltraFlavor>;
template <typename T>
concept IsGoblinFlavor = IsAnyOf<T, GoblinUltraFlavor,
GoblinUltraRecursiveFlavor_<UltraCircuitBuilder>,
GoblinUltraRecursiveFlavor_<GoblinUltraCircuitBuilder>>;
GoblinUltraRecursiveFlavor_<GoblinUltraCircuitBuilder>, GoblinUltraRecursiveFlavor_<CircuitSimulatorBN254>>;

template <typename T>
concept IsRecursiveFlavor = IsAnyOf<T, UltraRecursiveFlavor_<UltraCircuitBuilder>,
UltraRecursiveFlavor_<GoblinUltraCircuitBuilder>,
UltraRecursiveFlavor_<CircuitSimulatorBN254>,
GoblinUltraRecursiveFlavor_<UltraCircuitBuilder>,
GoblinUltraRecursiveFlavor_<GoblinUltraCircuitBuilder>>;
GoblinUltraRecursiveFlavor_<GoblinUltraCircuitBuilder>
,GoblinUltraRecursiveFlavor_<CircuitSimulatorBN254>>;


template <typename T> concept IsGrumpkinFlavor = IsAnyOf<T, ECCVMFlavor>;

template <typename T> concept IsFoldingFlavor = IsAnyOf<T, UltraFlavor,
GoblinUltraFlavor,
UltraRecursiveFlavor_<UltraCircuitBuilder>,
UltraRecursiveFlavor_<GoblinUltraCircuitBuilder>,
UltraRecursiveFlavor_<CircuitSimulatorBN254>,
GoblinUltraRecursiveFlavor_<UltraCircuitBuilder>,
GoblinUltraRecursiveFlavor_<GoblinUltraCircuitBuilder>>;
GoblinUltraRecursiveFlavor_<GoblinUltraCircuitBuilder>, GoblinUltraRecursiveFlavor_<CircuitSimulatorBN254>>;

template <typename Container, typename Element>
inline std::string flavor_get_label(Container&& container, const Element& element) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,17 @@ namespace bb::numeric {
* Implemented in terms of intrinsics which will use instructions such as `bsr` or `lzcnt` for best performance.
* Undefined behavior when input is 0.
*/
template <typename T> constexpr inline size_t count_leading_zeros(T const& u);

template <> constexpr inline size_t count_leading_zeros<uint32_t>(uint32_t const& u)
constexpr inline size_t count_leading_zeros(uint32_t const& u)
{
return static_cast<size_t>(__builtin_clz(u));
}

template <> constexpr inline size_t count_leading_zeros<uint64_t>(uint64_t const& u)
constexpr inline size_t count_leading_zeros(uint64_t const& u)
{
return static_cast<size_t>(__builtin_clzll(u));
}

template <> constexpr inline size_t count_leading_zeros<uint128_t>(uint128_t const& u)
constexpr inline size_t count_leading_zeros(uint128_t const& u)
{
auto hi = static_cast<uint64_t>(u >> 64);
if (hi != 0U) {
Expand All @@ -32,7 +30,7 @@ template <> constexpr inline size_t count_leading_zeros<uint128_t>(uint128_t con
return static_cast<size_t>(__builtin_clzll(lo)) + 64;
}

template <> constexpr inline size_t count_leading_zeros<uint256_t>(uint256_t const& u)
constexpr inline size_t count_leading_zeros(uint256_t const& u)
{
if (u.data[3] != 0U) {
return count_leading_zeros(u.data[3]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

namespace bb::merkle {
// TODO(Cody) Get rid of this?
enum HashType { FIXED_BASE_PEDERSEN, LOOKUP_PEDERSEN };
} // namespace bb::merkle
// TODO(https://github.com/AztecProtocol/barretenberg/issues/426)
enum HashType { FIXED_BASE_PEDERSEN, LOOKUP_PEDERSEN, NONE };
} // namespace bb::merkle
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

namespace bb::pedersen {
// TODO(Cody) Get rid of this?
enum CommitmentType { FIXED_BASE_PEDERSEN, LOOKUP_PEDERSEN };
} // namespace bb::pedersen
// TODO(https://github.com/AztecProtocol/barretenberg/issues/426)
enum CommitmentType { FIXED_BASE_PEDERSEN, LOOKUP_PEDERSEN, NONE };
} // namespace bb::pedersen
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
2) Precomputing for all possible size pairs is probably feasible and might be a better solution than instantiating
many instances separately. Then perhaps we could infer input type to `extend`.

3) There should be more thorough testing of this class in isolation.
*/
namespace bb {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,20 +567,22 @@ TYPED_TEST(ScalarMultiplicationTests, EndomorphismSplit)
using Fr = typename Curve::ScalarField;
using Fq = typename Curve::BaseField;

Fr scalar = Fr::random_element();
// NOTE: to appease GCC array-bounds checks, we need an extra Fr at the end of 'scalar'
// This is why it is an Fr[2]. Otherwise, GCC really doesn't like us type-punning in k2_t as it
// encompasses undefined memory (even though it doesn't use it).
Fr scalar[2];
scalar[0] = Fr::random_element();

Element expected = Group::one * scalar;
Element expected = Group::one * scalar[0];

// we want to test that we can split a scalar into two half-length components, using the same location in memory.
Fr* k1_t = &scalar;
Fr* k2_t = (Fr*)&scalar.data[2];
Fr* k1_t = &scalar[0];
Fr* k2_t = (Fr*)&scalar[0].data[2];

Fr::split_into_endomorphism_scalars(scalar, *k1_t, *k2_t);
Fr::split_into_endomorphism_scalars(scalar[0], *k1_t, *k2_t);
Fr k1{ (*k1_t).data[0], (*k1_t).data[1], 0, 0 };
Fr k2{ (*k2_t).data[0], (*k2_t).data[1], 0, 0 };
#if !defined(__clang__) && defined(__GNUC__)
#pragma GCC diagnostic pop
#endif

Element result;
Element t1 = Group::affine_one * k1;
AffineElement generator = Group::affine_one;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

using namespace bb;

using Builder = bb::UltraCircuitBuilder;

TEST(stdlib_aes128, encrypt_64_bytes)
{
typedef stdlib::field_t<UltraCircuitBuilder> field_pt;
typedef stdlib::witness_t<bb::UltraCircuitBuilder> witness_pt;
typedef stdlib::field_t<Builder> field_pt;
typedef stdlib::witness_t<Builder> witness_pt;

uint8_t key[16]{ 0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c };
uint8_t out[64]{ 0x76, 0x49, 0xab, 0xac, 0x81, 0x19, 0xb2, 0x46, 0xce, 0xe9, 0x8e, 0x9b, 0x12, 0xe9, 0x19, 0x7d,
Expand All @@ -32,7 +34,7 @@ TEST(stdlib_aes128, encrypt_64_bytes)
return converted;
};

auto builder = UltraCircuitBuilder();
auto builder = Builder();
maramihali marked this conversation as resolved.
Show resolved Hide resolved

std::vector<field_pt> in_field{
witness_pt(&builder, fr(convert_bytes(in))),
Expand Down
16 changes: 12 additions & 4 deletions barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -814,14 +814,21 @@ stdlib::byte_array<Builder> keccak<Builder>::hash(byte_array_ct& input, const ui

ASSERT(uint256_t(num_bytes.get_value()) <= input.size());

if (ctx == nullptr) {
// if buffer is constant compute hash and return w/o creating constraints
byte_array_ct output(nullptr, 32);
const auto constant_case = [&] { // if buffer is constant, compute hash and return w/o creating constraints
byte_array_ct output(nullptr, static_cast<uint32_t>(num_bytes.get_value() >> 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

The magic 32 wasn't good but I also don't understand why the bit shift is here and how that's supposed to be equivalent to what was there before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't recall adding this, but it has been a long time. Maybe it's an artifact of the merge? Does resetting to 32 cause tests to break? If not I would revert to the magic number.

const std::vector<uint8_t> result = hash_native(input.get_value());
for (size_t i = 0; i < 32; ++i) {
for (size_t i = 0; i < static_cast<uint32_t>(num_bytes.get_value() >> 1); ++i) {
output.set_byte(i, result[i]);
}
return output;
};

if constexpr (IsSimulator<Builder>) {
return constant_case();
}

if (ctx == nullptr) {
return constant_case();
}

// convert the input byte array into 64-bit keccak lanes (+ apply padding)
Expand Down Expand Up @@ -906,6 +913,7 @@ template <typename Builder> void generate_keccak_test_circuit(Builder& builder,
}
}

template class keccak<bb::CircuitSimulatorBN254>;
template class keccak<bb::UltraCircuitBuilder>;
template class keccak<bb::GoblinUltraCircuitBuilder>;
template void generate_keccak_test_circuit(bb::UltraCircuitBuilder&, size_t);
Expand Down
Loading
Loading