Skip to content

refactor: moving public inputs back to instance #5315

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

Merged
lucasxia01 merged 3 commits into from
Mar 20, 2024
Merged

refactor: moving public inputs back to instance #5315

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
4b2b6f7
moving public inputs back to instance
lucasxia01 Mar 19, 2024
10e649a
fix public inputs in oink verifier
lucasxia01 Mar 19, 2024
b69ffef
addressed comments
lucasxia01 Mar 20, 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
15 changes: 1 addition & 14 deletions barretenberg/cpp/src/barretenberg/flavor/goblin_ultra.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,20 +287,7 @@ class GoblinUltraFlavor {
* circuits.
* @todo TODO(https://github.com/AztecProtocol/barretenberg/issues/876)
*/
class VerificationKey : public VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey> {
public:
std::vector<FF> public_inputs;

VerificationKey(const size_t circuit_size, const size_t num_public_inputs)
: VerificationKey_(circuit_size, num_public_inputs)
{}

template <typename ProvingKeyPtr>
VerificationKey(const ProvingKeyPtr& proving_key)
: VerificationKey_(proving_key)
, public_inputs(proving_key->public_inputs)
{}
};
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

/**
* @brief A container for storing the partially evaluated multivariates produced by sumcheck.
Expand Down
6 changes: 0 additions & 6 deletions barretenberg/cpp/src/barretenberg/flavor/goblin_ultra_recursive.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ template <typename BuilderType> class GoblinUltraRecursiveFlavor_ {
class VerificationKey
: public VerificationKey_<GoblinUltraFlavor::PrecomputedEntities<Commitment>, VerifierCommitmentKey> {
public:
std::vector<FF> public_inputs;

VerificationKey(const size_t circuit_size, const size_t num_public_inputs)
{
this->circuit_size = circuit_size;
Expand All @@ -122,10 +120,6 @@ template <typename BuilderType> class GoblinUltraRecursiveFlavor_ {
this->log_circuit_size = numeric::get_msb(this->circuit_size);
this->num_public_inputs = native_key->num_public_inputs;
this->pub_inputs_offset = native_key->pub_inputs_offset;
this->public_inputs = std::vector<FF>(native_key->num_public_inputs);
for (auto [public_input, native_public_input] : zip_view(this->public_inputs, native_key->public_inputs)) {
public_input = FF::from_witness(builder, native_public_input);
}
this->q_m = Commitment::from_witness(builder, native_key->q_m);
this->q_l = Commitment::from_witness(builder, native_key->q_l);
this->q_r = Commitment::from_witness(builder, native_key->q_r);
Expand Down
15 changes: 1 addition & 14 deletions barretenberg/cpp/src/barretenberg/flavor/ultra.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,20 +290,7 @@ class UltraFlavor {
* that, and split out separate PrecomputedPolynomials/Commitments data for clarity but also for portability of our
* circuits.
*/
class VerificationKey : public VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey> {
public:
std::vector<FF> public_inputs;

VerificationKey(const size_t circuit_size, const size_t num_public_inputs)
: VerificationKey_(circuit_size, num_public_inputs)
{}

template <typename ProvingKeyPtr>
VerificationKey(const ProvingKeyPtr& proving_key)
: VerificationKey_(proving_key)
, public_inputs(proving_key->public_inputs)
{}
};
using VerificationKey = VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey>;

/**
* @brief A field element for each entity of the flavor. These entities represent the prover polynomials
Expand Down
6 changes: 0 additions & 6 deletions barretenberg/cpp/src/barretenberg/flavor/ultra_recursive.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,6 @@ template <typename BuilderType> class UltraRecursiveFlavor_ {
*/
class VerificationKey : public VerificationKey_<PrecomputedEntities<Commitment>, VerifierCommitmentKey> {
public:
std::vector<FF> public_inputs;

VerificationKey(const size_t circuit_size, const size_t num_public_inputs)
{
this->circuit_size = circuit_size;
Expand All @@ -293,10 +291,6 @@ template <typename BuilderType> class UltraRecursiveFlavor_ {
this->log_circuit_size = numeric::get_msb(this->circuit_size);
this->num_public_inputs = native_key->num_public_inputs;
this->pub_inputs_offset = native_key->pub_inputs_offset;
this->public_inputs = std::vector<FF>(native_key->num_public_inputs);
for (auto [public_input, native_public_input] : zip_view(this->public_inputs, native_key->public_inputs)) {
public_input = FF::from_witness(builder, native_public_input);
}
this->q_m = Commitment::from_witness(builder, native_key->q_m);
this->q_l = Commitment::from_witness(builder, native_key->q_l);
this->q_r = Commitment::from_witness(builder, native_key->q_r);
Expand Down
14 changes: 7 additions & 7 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 @@ -9,9 +9,10 @@ void ProtoGalaxyVerifier_<VerifierInstances>::receive_and_finalise_instance(cons
{
auto& key = inst->verification_key;
OinkVerifier<Flavor> oink_verifier{ key, transcript, domain_separator + '_' };
auto [relation_parameters, witness_commitments] = oink_verifier.verify();
inst->relation_parameters = relation_parameters;
inst->witness_commitments = witness_commitments;
auto [relation_parameters, witness_commitments, public_inputs] = oink_verifier.verify();
inst->relation_parameters = std::move(relation_parameters);
inst->witness_commitments = std::move(witness_commitments);
inst->public_inputs = std::move(public_inputs);

@lucasxia01 lucasxia01 Mar 19, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure if these std::move()s are doing anything here

@maramihali maramihali Mar 19, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

think it's good practice since we dont need the oink_verifier from here onwards and it makes transfer of ownership explicit


// Get the relation separation challenges
for (size_t idx = 0; idx < NUM_SUBRELATIONS - 1; idx++) {
Expand Down Expand Up @@ -96,16 +97,15 @@ std::shared_ptr<typename VerifierInstances::Instance> ProtoGalaxyVerifier_<Verif
vk_idx++;
}
next_accumulator->verification_key->num_public_inputs = accumulator->verification_key->num_public_inputs;
next_accumulator->verification_key->public_inputs =
std::vector<FF>(next_accumulator->verification_key->num_public_inputs, 0);
next_accumulator->public_inputs = std::vector<FF>(next_accumulator->verification_key->num_public_inputs, 0);
size_t public_input_idx = 0;
for (auto& public_input : next_accumulator->verification_key->public_inputs) {
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->verification_key->public_inputs[public_input_idx] * lagranges[inst];
public_input += instance->public_inputs[public_input_idx] * lagranges[inst];
inst++;
}
}
Expand Down
15 changes: 6 additions & 9 deletions ...rg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/protogalaxy_recursive_verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ void ProtoGalaxyRecursiveVerifier_<VerifierInstances>::receive_and_finalise_inst
transcript->template receive_from_prover<FF>(domain_separator + "_pub_inputs_offset");
inst->verification_key->pub_inputs_offset = uint32_t(pub_inputs_offset.get_value());

inst->verification_key->public_inputs.clear();
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);
inst->public_inputs.emplace_back(public_input_i);
}

// Get commitments to first three wire polynomials
Expand Down Expand Up @@ -72,7 +71,7 @@ void ProtoGalaxyRecursiveVerifier_<VerifierInstances>::receive_and_finalise_inst
transcript->template receive_from_prover<Commitment>(domain_separator + "_" + labels.z_lookup);

// Compute correction terms for grand products
const FF public_input_delta = compute_public_input_delta<Flavor>(inst->verification_key->public_inputs,
const FF public_input_delta = compute_public_input_delta<Flavor>(inst->public_inputs,
beta,
gamma,
inst->verification_key->circuit_size,
Expand Down Expand Up @@ -156,7 +155,7 @@ std::shared_ptr<typename VerifierInstances::Instance> ProtoGalaxyRecursiveVerifi
accumulator->verification_key->circuit_size, accumulator->verification_key->num_public_inputs);
next_accumulator->verification_key->pcs_verification_key = accumulator->verification_key->pcs_verification_key;
next_accumulator->verification_key->pub_inputs_offset = accumulator->verification_key->pub_inputs_offset;
next_accumulator->verification_key->public_inputs = accumulator->verification_key->public_inputs;
next_accumulator->public_inputs = accumulator->public_inputs;
size_t vk_idx = 0;
for (auto& expected_vk : next_accumulator->verification_key->get_all()) {
size_t inst = 0;
Expand Down Expand Up @@ -194,16 +193,14 @@ std::shared_ptr<typename VerifierInstances::Instance> ProtoGalaxyRecursiveVerifi
comm_idx++;
}

next_accumulator->verification_key->num_public_inputs = accumulator->verification_key->num_public_inputs;
next_accumulator->verification_key->public_inputs =
std::vector<FF>(next_accumulator->verification_key->num_public_inputs, 0);
next_accumulator->public_inputs = std::vector<FF>(next_accumulator->verification_key->num_public_inputs, 0);
size_t public_input_idx = 0;
for (auto& public_input : next_accumulator->verification_key->public_inputs) {
for (auto& public_input : next_accumulator->public_inputs) {
size_t inst = 0;
for (auto& instance : instances) {
if (instance->verification_key->num_public_inputs >=
next_accumulator->verification_key->num_public_inputs) {
public_input += instance->verification_key->public_inputs[public_input_idx] * lagranges[inst];
public_input += instance->public_inputs[public_input_idx] * lagranges[inst];
inst++;
};
}
Expand Down
12 changes: 6 additions & 6 deletions ...nberg/cpp/src/barretenberg/stdlib/honk_recursion/verifier/recursive_verifier_instance.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ template <IsRecursiveFlavor Flavor> class RecursiveVerifierInstance_ {
RelationParameters<FF> relation_parameters;
RelationSeparator alphas;
bool is_accumulator = false;
std::vector<FF> public_inputs;

// The folding parameters (\vec{β}, e) which are set for accumulators (i.e. relaxed instances).
std::vector<FF> gate_challenges;
Expand All @@ -48,13 +49,13 @@ template <IsRecursiveFlavor Flavor> class RecursiveVerifierInstance_ {
: verification_key(std::make_shared<VerificationKey>(instance->verification_key->circuit_size,
instance->verification_key->num_public_inputs))
, is_accumulator(bool(instance->is_accumulator))
, public_inputs(std::vector<FF>(instance->verification_key->num_public_inputs))
{

verification_key->pub_inputs_offset = instance->verification_key->pub_inputs_offset;
verification_key->pcs_verification_key = instance->verification_key->pcs_verification_key;
verification_key->public_inputs = std::vector<FF>(instance->verification_key->num_public_inputs);
for (auto [public_input, native_public_input] :
zip_view(verification_key->public_inputs, instance->verification_key->public_inputs)) {

for (auto [public_input, native_public_input] : zip_view(public_inputs, instance->public_inputs)) {
public_input = FF::from_witness(builder, native_public_input);
}

Expand Down Expand Up @@ -110,9 +111,8 @@ template <IsRecursiveFlavor Flavor> class RecursiveVerifierInstance_ {
VerifierInstance inst(inst_verification_key);
inst.is_accumulator = is_accumulator;

inst.verification_key->public_inputs = std::vector<NativeFF>(verification_key->num_public_inputs);
for (auto [public_input, inst_public_input] :
zip_view(verification_key->public_inputs, inst.verification_key->public_inputs)) {
inst.public_inputs = std::vector<NativeFF>(verification_key->num_public_inputs);
for (auto [public_input, inst_public_input] : zip_view(public_inputs, inst.public_inputs)) {
inst_public_input = public_input.get_value();
}

Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/sumcheck/instance/verifier_instance.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ template <class Flavor, size_t NUM_ = 2> class VerifierInstance_ {
RelationParameters<FF> relation_parameters;
RelationSeparator alphas;
bool is_accumulator = false;
std::vector<FF> public_inputs;

// The folding parameters (\vec{β}, e) which are set for accumulators (i.e. relaxed instances).
std::vector<FF> gate_challenges;
Expand Down
17 changes: 6 additions & 11 deletions barretenberg/cpp/src/barretenberg/ultra_honk/oink_verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ template <IsUltraFlavor Flavor> OinkOutput<Flavor> OinkVerifier<Flavor>::verify(
execute_log_derivative_inverse_round();
execute_grand_product_computation_round();

return OinkOutput<Flavor>{
.relation_parameters = relation_parameters,
.commitments = witness_comms,
};
return OinkOutput<Flavor>{ .relation_parameters = std::move(relation_parameters),
.commitments = std::move(witness_comms),
.public_inputs = std::move(public_inputs) };
Comment thread
lucasxia01 marked this conversation as resolved.
Outdated
}

/**
Expand All @@ -40,11 +39,10 @@ template <IsUltraFlavor Flavor> void OinkVerifier<Flavor>::execute_preamble_roun
ASSERT(public_input_size == key->num_public_inputs);
ASSERT(pub_inputs_offset == key->pub_inputs_offset);

key->public_inputs.clear();
for (size_t i = 0; i < public_input_size; ++i) {
auto public_input_i =
transcript->template receive_from_prover<FF>(domain_separator + "public_input_" + std::to_string(i));
key->public_inputs.emplace_back(public_input_i);
public_inputs.emplace_back(public_input_i);
}
}

Expand Down Expand Up @@ -116,11 +114,8 @@ template <IsUltraFlavor Flavor> void OinkVerifier<Flavor>::execute_log_derivativ
*/
template <IsUltraFlavor Flavor> void OinkVerifier<Flavor>::execute_grand_product_computation_round()
{
const FF public_input_delta = compute_public_input_delta<Flavor>(key->public_inputs,
relation_parameters.beta,
relation_parameters.gamma,
key->circuit_size,
key->pub_inputs_offset);
const FF public_input_delta = compute_public_input_delta<Flavor>(
public_inputs, relation_parameters.beta, relation_parameters.gamma, key->circuit_size, key->pub_inputs_offset);
const FF lookup_grand_product_delta =
compute_lookup_grand_product_delta<FF>(relation_parameters.beta, relation_parameters.gamma, key->circuit_size);

Expand Down
2 changes: 2 additions & 0 deletions barretenberg/cpp/src/barretenberg/ultra_honk/oink_verifier.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace bb {
template <IsUltraFlavor Flavor> struct OinkOutput {
bb::RelationParameters<typename Flavor::FF> relation_parameters;
typename Flavor::WitnessCommitments commitments;
std::vector<typename Flavor::FF> public_inputs;
};

/**
Expand All @@ -36,6 +37,7 @@ template <IsUltraFlavor Flavor> class OinkVerifier {
typename Flavor::CommitmentLabels comm_labels;
bb::RelationParameters<FF> relation_parameters;
WitnessCommitments witness_comms;
std::vector<FF> public_inputs;

OinkVerifier(const std::shared_ptr<VerificationKey>& verifier_key,
const std::shared_ptr<Transcript>& transcript,
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -49,7 +49,7 @@ template <typename Flavor> bool UltraVerifier_<Flavor>::verify_proof(const HonkP
transcript = std::make_shared<Transcript>(proof);
VerifierCommitments commitments{ key };
OinkVerifier<Flavor> oink_verifier{ key, transcript };
auto [relation_parameters, witness_commitments] = oink_verifier.verify();
auto [relation_parameters, witness_commitments, public_inputs] = oink_verifier.verify();

@lucasxia01 lucasxia01 Mar 19, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

seems like public_inputs isn't used by ultra_verifier so maybe this can just be _

@maramihali maramihali Mar 19, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the public inputs are used by the verifier to compute the public_input_delta which i believe is now computed by OinkVerifier so maybe the Oink output shouldn't even include them

@lucasxia01 lucasxia01 Mar 19, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

they're included because the protogalaxy verifier needs to fold them

@maramihali maramihali Mar 19, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Right yeah


// Copy the witness_commitments over to the VerifierCommitments
for (auto [wit_comm_1, wit_comm_2] : zip_view(commitments.get_witness(), witness_commitments.get_all())) {
Expand Down