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

chore: make first iteration of protogalaxy more efficient #4630

Merged
merged 11 commits into from
Feb 20, 2024
Merged

chore: make first iteration of protogalaxy more efficient #4630

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
76202c4
initial tinkering
lucasxia01 Feb 15, 2024
7a03e7c
Merge remote-tracking branch 'origin/master' into lx/protogalaxy-firs…
lucasxia01 Feb 15, 2024
37151e6
test failing?
lucasxia01 Feb 15, 2024
18ed4ac
zero-ing first accumulator to avoid generating beta
lucasxia01 Feb 15, 2024
a1e7506
Merge remote-tracking branch 'origin/master' into lx/protogalaxy-firs…
lucasxia01 Feb 15, 2024
72724fe
avoid perturbator computation
lucasxia01 Feb 15, 2024
00f8217
forgot to commit a file
lucasxia01 Feb 15, 2024
0bfb753
updates based on comments
lucasxia01 Feb 16, 2024
7bdf9bb
adding back the gate_challenges initialization
lucasxia01 Feb 16, 2024
8efa399
small update
lucasxia01 Feb 19, 2024
46d142e
small upd to address comments
lucasxia01 Feb 19, 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
19 changes: 7 additions & 12 deletions barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,17 +154,8 @@ template <class ProverInstances> void ProtoGalaxyProver_<ProverInstances>::prepa
send_accumulator(instance, domain_separator);
} else {
// This is the first round of folding and we need to generate some gate challenges.
// TODO(https://github.com/AztecProtocol/barretenberg/issues/740): implement option 2 to make this more
// efficient by avoiding the computation of the perturbator
finalise_and_send_instance(instance, domain_separator);
instance->target_sum = 0;
auto beta = transcript->template get_challenge<FF>(domain_separator + "_initial_gate_challenge");
std::vector<FF> gate_challenges(instance->log_instance_size);
gate_challenges[0] = beta;
for (size_t i = 1; i < instance->log_instance_size; i++) {
gate_challenges[i] = gate_challenges[i - 1].sqr();
}
instance->gate_challenges = gate_challenges;
}

idx++;
Expand Down Expand Up @@ -315,9 +306,13 @@ template <class ProverInstances> void ProtoGalaxyProver_<ProverInstances>::pertu
state.accumulator = get_accumulator();
FF delta = transcript->template get_challenge<FF>("delta");
state.deltas = compute_round_challenge_pows(state.accumulator->log_instance_size, delta);
state.perturbator = compute_perturbator(state.accumulator, state.deltas);
for (size_t idx = 0; idx <= state.accumulator->log_instance_size; idx++) {
transcript->send_to_verifier("perturbator_" + std::to_string(idx), state.perturbator[idx]);
if (state.accumulator->is_accumulator) {
state.perturbator = compute_perturbator(state.accumulator, state.deltas);
for (size_t idx = 0; idx <= state.accumulator->log_instance_size; idx++) {
transcript->send_to_verifier("perturbator_" + std::to_string(idx), state.perturbator[idx]);
}
} else {
state.perturbator = Polynomial<FF>(state.accumulator->log_instance_size + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

you could do this first and then if accumulator->is_accumulator is true you run compute_perturbator which is easier to read

Copy link
Contributor Author

@lucasxia01 lucasxia01 Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

isn't that extra computation?

Copy link
Contributor

Choose a reason for hiding this comment

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

I generally prefer readability in cases like this when it’s basically negligible

}
};

Expand Down
2 changes: 0 additions & 2 deletions barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@ template <class ProverInstances_> class ProtoGalaxyProver_ {

// Returns the accumulator, which is the first element in ProverInstances. The accumulator is assumed to have the
// FoldingParameters set and be the result of a previous round of folding.
// TODO(https://github.com/AztecProtocol/barretenberg/issues/740): handle the case when the accumulator is empty
// (i.e. we are in the first round of folding)/
std::shared_ptr<Instance> get_accumulator() { return instances[0]; }
lucasxia01 marked this conversation as resolved.
Show resolved Hide resolved

/**
Expand Down
18 changes: 6 additions & 12 deletions barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,8 @@ void ProtoGalaxyVerifier_<VerifierInstances>::prepare_for_folding(const std::vec
receive_accumulator(inst, domain_separator);
} else {
// This is the first round of folding and we need to generate some gate challenges.
// TODO(https://github.com/AztecProtocol/barretenberg/issues/740): implement option 2 to make this more
// efficient by avoiding the computation of the perturbator
receive_and_finalise_instance(inst, domain_separator);
inst->target_sum = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

could also maybe get rid of this, but its not that important since its one value

Copy link
Contributor

Choose a reason for hiding this comment

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

That’s the issue you are resolving I belive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, sorry, its referring to the target_sum

Copy link
Contributor

@maramihali maramihali Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No you cannot do that, the verifier should initialise the first coefficient of the perturbator with the target sum of the accumulator (i’m gonna make that more explicit in the follow up

auto beta = transcript->template get_challenge<FF>(domain_separator + "_initial_gate_challenge");
std::vector<FF> gate_challenges(inst->log_instance_size);
gate_challenges[0] = beta;
for (size_t i = 1; i < inst->log_instance_size; i++) {
gate_challenges[i] = gate_challenges[i - 1].sqr();
}
inst->gate_challenges = gate_challenges;
}
index++;

Expand All @@ -189,9 +180,12 @@ bool ProtoGalaxyVerifier_<VerifierInstances>::verify_folding_proof(const std::ve
auto accumulator = get_accumulator();
auto deltas = compute_round_challenge_pows(accumulator->log_instance_size, delta);

std::vector<FF> perturbator_coeffs(accumulator->log_instance_size + 1);
for (size_t idx = 0; idx <= accumulator->log_instance_size; idx++) {
perturbator_coeffs[idx] = transcript->template receive_from_prover<FF>("perturbator_" + std::to_string(idx));
std::vector<FF> perturbator_coeffs(accumulator->log_instance_size + 1, 0);
if (accumulator->is_accumulator) {
for (size_t idx = 0; idx <= accumulator->log_instance_size; idx++) {
perturbator_coeffs[idx] =
transcript->template receive_from_prover<FF>("perturbator_" + std::to_string(idx));
}
}

if (perturbator_coeffs[0] != accumulator->target_sum) {
Expand Down
18 changes: 6 additions & 12 deletions ...rg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/protogalaxy_recursive_verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,17 +164,8 @@ template <class VerifierInstances> void ProtoGalaxyRecursiveVerifier_<VerifierIn
receive_accumulator(inst, domain_separator);
} else {
// This is the first round of folding and we need to generate some gate challenges.
// TODO(https://github.com/AztecProtocol/barretenberg/issues/740): implement option 2 to make this more
// efficient by avoiding the computation of the perturbator
receive_and_finalise_instance(inst, domain_separator);
inst->target_sum = 0;
auto beta = transcript->template get_challenge<FF>(domain_separator + "_initial_gate_challenge");
std::vector<FF> gate_challenges(inst->log_instance_size);
gate_challenges[0] = beta;
for (size_t i = 1; i < inst->log_instance_size; i++) {
gate_challenges[i] = gate_challenges[i - 1].sqr();
}
inst->gate_challenges = gate_challenges;
}
index++;

Expand All @@ -199,9 +190,12 @@ void ProtoGalaxyRecursiveVerifier_<VerifierInstances>::verify_folding_proof(cons
auto accumulator = get_accumulator();
auto deltas = compute_round_challenge_pows(accumulator->log_instance_size, delta);

std::vector<FF> perturbator_coeffs(accumulator->log_instance_size + 1);
for (size_t idx = 0; idx <= accumulator->log_instance_size; idx++) {
perturbator_coeffs[idx] = transcript->template receive_from_prover<FF>("perturbator_" + std::to_string(idx));
std::vector<FF> perturbator_coeffs(accumulator->log_instance_size + 1, 0);
if (accumulator->is_accumulator) {
for (size_t idx = 0; idx <= accumulator->log_instance_size; idx++) {
perturbator_coeffs[idx] =
transcript->template receive_from_prover<FF>("perturbator_" + std::to_string(idx));
}
}

// TODO(https://github.com/AztecProtocol/barretenberg/issues/833): As currently the stdlib transcript is not
Expand Down
Loading