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.

Sign up for GitHub

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: protogalaxy interfaces #2125

Merged
merged 31 commits into from
Sep 15, 2023
Merged

feat: protogalaxy interfaces #2125

Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
94418e5
wip
maramihali Sep 6, 2023
f98e95f
more wip
maramihali Sep 7, 2023
0749289
even more wip
maramihali Sep 7, 2023
06c02ac
more wip
maramihali Sep 8, 2023
884318a
more wip
maramihali Sep 8, 2023
6403b3b
more wip
maramihali Sep 8, 2023
18e4019
finding bug
maramihali Sep 10, 2023
9172e6f
more interface
maramihali Sep 11, 2023
2018ffe
more wip
maramihali Sep 11, 2023
9ff0c23
wip
maramihali Sep 11, 2023
c76f0cd
remove unwanted lines
maramihali Sep 11, 2023
8c38123
potentially fix test-ci
maramihali Sep 11, 2023
34a1da5
debug
maramihali Sep 11, 2023
88ea704
check crs_factor is null before initialising it
maramihali Sep 12, 2023
1560d24
fix bug
maramihali Sep 12, 2023
fc4880f
remove info statements
maramihali Sep 12, 2023
4e11ee8
cleanup of namings
maramihali Sep 13, 2023
67ace3b
cleanup
maramihali Sep 13, 2023
cf4435f
remove prover_library
maramihali Sep 13, 2023
c7a6282
add instance.test.cpp that i forgot to add
maramihali Sep 13, 2023
0157a1c
add grand_product_library.test.cpp that i forgot to add
maramihali Sep 13, 2023
62e0c62
chore: move bb for merge
ludamad0 Sep 13, 2023
b5de3a9
chore: remove build-system for merge
ludamad0 Sep 13, 2023
8f07bd4
Merge
ludamad0 Sep 13, 2023
7e3bf57
resolve merge conflicts
maramihali Sep 13, 2023
78a643b
actually delete prover_library and resolve comments
maramihali Sep 14, 2023
1eb15fb
address some review comments
maramihali Sep 14, 2023
4fba3a2
start work to make standard honk support instances
maramihali Sep 14, 2023
ce7d33c
continue work for standard honk with instances
maramihali Sep 15, 2023
7406e65
Merge branch 'master' into mm/pg-interfaces
maramihali Sep 15, 2023
599d20a
Merge branch 'master' into mm/pg-interfaces
maramihali Sep 15, 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
18 changes: 14 additions & 4 deletions barretenberg/cpp/src/barretenberg/benchmark/honk_bench/benchmark_utilities.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,21 @@ void construct_proof_with_specified_num_iterations(State& state,
test_circuit_function(builder, num_iterations);

auto composer = Composer();
auto ext_prover = composer.create_prover(builder);
state.ResumeTiming();
if constexpr (Composer::NAME_STRING == "UltraHonk") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the type system for this (one of the Is concepts) and not the name string. I think the name string can go away from the Instance definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to get rid of the NAME_STRING altogether but it was needed for something. But in the past using identifiers lead to really confusing code.

auto instance = composer.create_instance(builder);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ideally, I think the benchmarks should be templated by Flavor

auto ext_prover = composer.create_prover(instance);
state.ResumeTiming();

// Construct proof
auto proof = ext_prover.construct_proof();
// Construct proof
auto proof = ext_prover.construct_proof();

} else {
auto ext_prover = composer.create_prover(builder);
Copy link
Contributor

Choose a reason for hiding this comment

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

How much more work would it be to implement for StandardHonk? I know we talked about this and I suggested doing Ultra only, but now that I see it leads a major divergence in structure and also in interace, I'm in favor of bringing Standard along for the ride. Should be routine now that you've done the same for Ultra, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

erm, couple of hours max, will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but the divergence around here is between honk and plonk now

Copy link
Contributor

Choose a reason for hiding this comment

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

Not ideal, but we have accepted divergence with Plonk if it means improving Honk.

state.ResumeTiming();

// Construct proof
auto proof = ext_prover.construct_proof();
}
}
}

Expand Down
9 changes: 6 additions & 3 deletions barretenberg/cpp/src/barretenberg/honk/composer/goblin_ultra_composer.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ class GoblinUltraHonkComposerTests : public ::testing::Test {
*/
TEST_F(GoblinUltraHonkComposerTests, SimpleCircuit)
{
auto builder = UltraCircuitBuilder();
using fr = barretenberg::fr;
using g1 = barretenberg::g1;
auto builder = proof_system::UltraCircuitBuilder();

// Define an arbitrary number of operations/gates
size_t num_ecc_ops = 3;
Expand Down Expand Up @@ -55,8 +57,9 @@ TEST_F(GoblinUltraHonkComposerTests, SimpleCircuit)
}

auto composer = GoblinUltraComposer();
auto prover = composer.create_prover(builder);
auto verifier = composer.create_verifier(builder);
auto instance = composer.create_instance(builder);
auto prover = composer.create_prover(instance);
auto verifier = composer.create_verifier(instance);
auto proof = prover.construct_proof();
bool verified = verifier.verify_proof(proof);
EXPECT_EQ(verified, true);
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/honk/composer/standard_composer.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ using namespace proof_system::honk;
#define TYPE_ALIASES \
using Flavor = TypeParam; \
using FF = typename Flavor::FF; \
using CircuitBuilder = StandardCircuitBuilder_<FF>; \
using CircuitBuilder = proof_system::StandardCircuitBuilder_<FF>; \
using Composer = StandardComposer_<Flavor>;

namespace test_standard_honk_composer {
Expand All @@ -28,7 +28,7 @@ template <typename Flavor> class StandardHonkComposerTests : public ::testing::T
// TODO(640): The Standard Honk on Grumpkin test suite fails unless the SRS is initialised for every test.
virtual void SetUp()
{
if constexpr (IsGrumpkinFlavor<Flavor>) {
if constexpr (proof_system::IsGrumpkinFlavor<Flavor>) {
barretenberg::srs::init_grumpkin_crs_factory("../srs_db/grumpkin");
} else {
barretenberg::srs::init_crs_factory("../srs_db/ignition");
Expand Down
Loading