Skip to content

fix: set client ivc test vkeys properly #5230

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

Closed
lucasxia01 wants to merge 1 commit into from
Closed

fix: set client ivc test vkeys properly #5230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,13 @@ TEST_F(ClientIVCTests, Full)
auto intermediary_acc = update_accumulator_and_decide_native(
ivc.prover_fold_output.accumulator, kernel_fold_proof, foo_verifier_instance, kernel_vk);

VerifierFoldData kernel_fold_output = { kernel_fold_proof, function_vk_with_merge };
VerifierFoldData kernel_fold_output = { kernel_fold_proof, kernel_vk };
Copy link
Contributor Author

@lucasxia01 lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

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

kernel_vk is the same as function_vk_with_merge here but its the correct one name-wise

size_t NUM_CIRCUITS = 1;
for (size_t circuit_idx = 0; circuit_idx < NUM_CIRCUITS; ++circuit_idx) {
// Accumulate function circuit
Builder function_circuit = create_mock_circuit(ivc);
FoldProof function_fold_proof = ivc.accumulate(function_circuit);
function_vk_with_merge = std::make_shared<VerificationKey>(ivc.prover_instance->proving_key);
Copy link
Contributor Author

@lucasxia01 lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

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

we used to never reset function_vk_with_merge, so we were using the same vk for all function circuits, which worked generally since all of our function circuits were the same except for the public inputs and the witness values

Copy link

@maramihali maramihali Mar 15, 2024

Choose a reason for hiding this comment

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

I believe there is no need to reset the vk? the witness values and public inputs are not relevant for the vk. Am I missing something?

Copy link

@maramihali maramihali Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

oh, right, the VerificationKey contains the public inputs.. but still the number of those should not change across function circuits in this test, hmm.. Also are we certain we want the PIs to be part of the VK?


intermediary_acc = update_accumulator_and_decide_native(
ivc.prover_fold_output.accumulator, function_fold_proof, intermediary_acc, function_vk_with_merge);
Expand Down
5 changes: 3 additions & 2 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 @@ -15,11 +15,12 @@ void ProtoGalaxyVerifier_<VerifierInstances>::receive_and_finalise_instance(cons
transcript->template receive_from_prover<uint32_t>(domain_separator + "_public_input_size");
inst->verification_key->pub_inputs_offset =
transcript->template receive_from_prover<uint32_t>(domain_separator + "_pub_inputs_offset");
inst->verification_key->public_inputs.clear();
std::vector<FF> public_inputs;
for (size_t i = 0; i < inst->verification_key->num_public_inputs; ++i) {
auto public_input_i =
transcript->template receive_from_prover<FF>(domain_separator + "_public_input_" + std::to_string(i));
inst->verification_key->public_inputs.emplace_back(public_input_i);
public_inputs.emplace_back(public_input_i);
ASSERT(public_input_i == inst->verification_key->public_inputs[i]);
Copy link
Contributor Author

@lucasxia01 lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

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

actually check that the public inputs match the verification key.

Copy link
Contributor Author

@lucasxia01 lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

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

this uncovered the ivc test bug

}

// Get commitments to first three wire polynomials
Expand Down
3 changes: 3 additions & 0 deletions barretenberg/cpp/src/barretenberg/ultra_honk/ultra_verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ template <typename Flavor> bool UltraVerifier_<Flavor>::verify_proof(const HonkP
for (size_t i = 0; i < public_input_size; ++i) {
auto public_input_i = transcript->template receive_from_prover<FF>("public_input_" + std::to_string(i));
public_inputs.emplace_back(public_input_i);
if (public_input_i != key->public_inputs[i]) {
return false;
Copy link

@maramihali maramihali Mar 15, 2024

Choose a reason for hiding this comment

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

can you please add a comment here?

}
}

// Get commitments to first three wire polynomials
Expand Down