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

Jump to bottom

chore: Remove unneeded public input folding #7094

Merged
merged 3 commits into from
Jun 19, 2024
Merged

chore: Remove unneeded public input folding #7094

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22db0c5
remove pi folding and add a test
ledwards2225 Jun 18, 2024
b3ac6d6
remove pi folding from verifier as well
ledwards2225 Jun 18, 2024
55e28ec
pub inputs still needed in some places for now
ledwards2225 Jun 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
33 changes: 33 additions & 0 deletions barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,34 @@ template <typename Flavor> class ProtoGalaxyTests : public testing::Test {
}
}

/**
* @brief Testing one valid round of folding followed by the decider.
* @brief For additional robustness we give one of the circuits more public inputs than the other
*
*/
static void test_full_protogalaxy_simple()
{
// Construct a first circuit with some public inputs
Builder builder1;
construct_circuit(builder1);
bb::MockCircuits::add_arithmetic_gates_with_public_inputs(builder1, /*num_gates=*/4);

// Construct a second circuit with no public inputs
Builder builder2;
construct_circuit(builder2);

// Construct the prover/verifier instances for each
TupleOfInstances instances;
construct_prover_and_verifier_instance(instances, builder1);
construct_prover_and_verifier_instance(instances, builder2);

// Perform prover and verifier folding
auto [prover_accumulator, verifier_accumulator] = fold_and_verify(get<0>(instances), get<1>(instances));
check_accumulator_target_sum_manual(prover_accumulator, true);

decide_and_verify(prover_accumulator, verifier_accumulator, true);
}

/**
* @brief Testing two valid rounds of folding followed by the decider.
*
Expand Down Expand Up @@ -489,6 +517,11 @@ TYPED_TEST(ProtoGalaxyTests, CombineAlpha)
TestFixture::test_combine_alpha();
}

TYPED_TEST(ProtoGalaxyTests, FullProtogalaxySimple)
{
TestFixture::test_full_protogalaxy_simple();
}

TYPED_TEST(ProtoGalaxyTests, FullProtogalaxyTest)
{
TestFixture::test_full_protogalaxy();
Expand Down
16 changes: 0 additions & 16 deletions barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,22 +104,6 @@ std::shared_ptr<typename ProverInstances::Instance> ProtoGalaxyProver_<ProverIns
});
}

// Fold the public inputs and send to the verifier
size_t el_idx = 0;
for (auto& el : next_accumulator->proving_key.public_inputs) {
el *= lagranges[0];
size_t inst = 0;
for (size_t inst_idx = 1; inst_idx < ProverInstances::NUM; inst_idx++) {
auto& instance = instances[inst_idx];
// TODO(https://github.com/AztecProtocol/barretenberg/issues/830)
if (instance->proving_key.num_public_inputs >= next_accumulator->proving_key.num_public_inputs) {
el += instance->proving_key.public_inputs[el_idx] * lagranges[inst];
inst++;
};
}
el_idx++;
}

// Evaluate the combined batching α_i univariate at challenge to obtain next α_i and send it to the
// verifier, where i ∈ {0,...,NUM_SUBRELATIONS - 1}
auto& folded_alphas = next_accumulator->alphas;
Expand Down
14 changes: 1 addition & 13 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 @@ -115,19 +115,7 @@ std::shared_ptr<typename VerifierInstances::Instance> ProtoGalaxyVerifier_<Verif
next_accumulator->verification_key->num_public_inputs = accumulator->verification_key->num_public_inputs;
next_accumulator->public_inputs =
std::vector<FF>(static_cast<size_t>(next_accumulator->verification_key->num_public_inputs), 0);
size_t public_input_idx = 0;
for (auto& public_input : next_accumulator->public_inputs) {
size_t inst = 0;
for (auto& instance : instances) {
// TODO(https://github.com/AztecProtocol/barretenberg/issues/830)
if (instance->verification_key->num_public_inputs >=
next_accumulator->verification_key->num_public_inputs) {
public_input += instance->public_inputs[public_input_idx] * lagranges[inst];
inst++;
}
}
public_input_idx++;
}

Copy link
Contributor

@maramihali maramihali Jun 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There is also no reason to have the line above or, in fact, the public_inputs data member in prover and verifier instance. Would you please remove them?

Copy link
Contributor

@maramihali maramihali Jun 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also in the protogalaxy recursive verifier code

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 removed a bit more but I'm leaving the pub inputs member in the verifier instance for now since its used in the PG rec verifier for the PI delta. I don't want this to spiral too much. Probably the PG reg verifier should be using oink like the native verifier?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, thanks for this!

next_accumulator->is_accumulator = true;

// Compute next folding parameters
Expand Down
Loading