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.

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

Cg/arithmetization #296

Merged
merged 11 commits into from
Mar 31, 2023
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
.vscode
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The settings.json and c_cpp_properties.json files are already being tracked, so having this in the .gitignore does nothing. We should probably remove these and .gitignore to allow people overriding code-workspace settings, or we should start using multiple code-workspace files.

result
node_modules
.yarn/cache
Expand Down
6 changes: 3 additions & 3 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@
"typescript.enablePromptUseWorkspaceTsdk": true,
"eslint.nodePath": "ts/.yarn/sdks",
"prettier.prettierPath": "ts/.yarn/sdks/prettier/index.js",
"[cpp]": {
"editor.defaultFormatter": "ms-vscode.cpptools"
"[cpp]": { // doesn't conflict with barratenberg.code-workspace settings.
"editor.defaultFormatter": "llvm-vs-code-extensions.vscode-clangd"
},
"[terraform]": {
"editor.defaultFormatter": "hashicorp.terraform"
Expand All @@ -128,4 +128,4 @@
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"cmake.sourceDirectory": "${workspaceFolder}/cpp"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ std::shared_ptr<bonk::proving_key> StandardHonkComposerHelper<CircuitConstructor
StandardHonkComposerHelper::compute_proving_key_base(circuit_constructor, plonk::ComposerType::STANDARD_HONK);

// Compute sigma polynomials (we should update that late)
compute_standard_honk_sigma_permutations<CircuitConstructor::program_width>(circuit_constructor,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since in this PR there is now an array wires, it made sense to finally change to the more descriptive name num_wires.

circuit_proving_key.get());
compute_standard_honk_id_polynomials<CircuitConstructor::program_width>(circuit_proving_key.get());
compute_standard_honk_sigma_permutations<CircuitConstructor::num_wires>(circuit_constructor,
circuit_proving_key.get());
compute_standard_honk_id_polynomials<CircuitConstructor::num_wires>(circuit_proving_key.get());

compute_first_and_last_lagrange_polynomials(circuit_proving_key.get());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "barretenberg/honk/pcs/commitment_key.hpp"
#include "barretenberg/proof_system/verification_key/verification_key.hpp"
#include "barretenberg/plonk/proof_system/verifier/verifier.hpp"
#include "barretenberg/proof_system/composer/composer_base.hpp"
#include "barretenberg/proof_system/arithmetization/gate_data.hpp"
#include "barretenberg/proof_system/composer/composer_helper_lib.hpp"
#include "barretenberg/proof_system/composer/permutation_helper.hpp"

Expand All @@ -21,7 +21,7 @@ namespace honk {
template <typename CircuitConstructor> class StandardHonkComposerHelper {
public:
static constexpr size_t NUM_RANDOMIZED_GATES = 2; // equal to the number of multilinear evaluations leaked
static constexpr size_t program_width = CircuitConstructor::program_width;
static constexpr size_t num_wires = CircuitConstructor::num_wires;
std::shared_ptr<bonk::proving_key> circuit_proving_key;
std::vector<barretenberg::polynomial> wire_polynomials;
std::shared_ptr<bonk::verification_key> circuit_verification_key;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class StandardHonkComposer {

// Leaving it in for now just in case
bool contains_recursive_proof = false;
static constexpr size_t program_width = STANDARD_WIDTH;
static constexpr size_t num_wires = StandardCircuitConstructor::num_wires;

/**Standard methods*/

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ TEST(StandardHonkComposer, SigmaIDCorrectness)
barretenberg::fr right = barretenberg::fr::one();

// Let's check that indices are the same and nothing is lost, first
for (size_t j = 0; j < composer.program_width; ++j) {
for (size_t j = 0; j < composer.num_wires; ++j) {
std::string index = std::to_string(j + 1);
const auto& sigma_j = proving_key->polynomial_store.get("sigma_" + index + "_lagrange");
for (size_t i = 0; i < n; ++i) {
Expand All @@ -66,7 +66,7 @@ TEST(StandardHonkComposer, SigmaIDCorrectness)
// Now let's check that witness values correspond to the permutation
composer.compute_witness();

for (size_t j = 0; j < composer.program_width; ++j) {
for (size_t j = 0; j < composer.num_wires; ++j) {
std::string index = std::to_string(j + 1);
const auto& permutation_polynomial = proving_key->polynomial_store.get("sigma_" + index + "_lagrange");
const auto& witness_polynomial = composer.composer_helper.wire_polynomials[j];
Expand Down Expand Up @@ -207,11 +207,11 @@ TEST(StandardHonkComposer, AssertEquals)
auto get_maximum_cycle = [](auto& composer) {
// Compute the proving key for sigma polynomials
auto proving_key = composer.compute_proving_key();
auto permutation_length = composer.program_width * proving_key->circuit_size;
auto permutation_length = composer.num_wires * proving_key->circuit_size;
std::vector<polynomial> sigma_polynomials;

// Put the sigma polynomials into a vector for easy access
for (size_t i = 0; i < composer.program_width; i++) {
for (size_t i = 0; i < composer.num_wires; i++) {
std::string index = std::to_string(i + 1);
sigma_polynomials.push_back(proving_key->polynomial_store.get("sigma_" + index + "_lagrange"));
}
Expand Down Expand Up @@ -301,7 +301,7 @@ TEST(StandardHonkComposer, VerificationKeyCreation)
// committed to, we simply check that the verification key now contains the appropriate number of constraint and
// permutation selector commitments. This method should work with any future arithemtization.
EXPECT_EQ(verification_key->commitments.size(),
composer.circuit_constructor.selectors.size() + composer.program_width * 2 + 2);
composer.circuit_constructor.selectors.size() + composer.num_wires * 2 + 2);
}

/**
Expand All @@ -316,7 +316,7 @@ TEST(StandardHonkComposer, SumcheckRelationCorrectness)
{
// Create a composer and a dummy circuit with a few gates
StandardHonkComposer composer = StandardHonkComposer();
static const size_t program_width = StandardHonkComposer::program_width;
static const size_t num_wires = StandardHonkComposer::num_wires;
fr a = fr::one();
// Using the public variable to check that public_input_delta is computed and added to the relation correctly
uint32_t a_idx = composer.add_public_variable(a);
Expand Down Expand Up @@ -350,8 +350,8 @@ TEST(StandardHonkComposer, SumcheckRelationCorrectness)

constexpr size_t num_polynomials = honk::StandardArithmetization::NUM_POLYNOMIALS;
// Compute grand product polynomial
polynomial z_perm_poly = prover_library::compute_permutation_grand_product<program_width>(
prover.key, prover.wire_polynomials, beta, gamma);
polynomial z_perm_poly =
prover_library::compute_permutation_grand_product<num_wires>(prover.key, prover.wire_polynomials, beta, gamma);

// Create an array of spans to the underlying polynomials to more easily
// get the transposition.
Expand Down
6 changes: 4 additions & 2 deletions cpp/src/barretenberg/honk/flavor/flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <array>
#include <string>
#include "barretenberg/common/log.hpp"
#include "barretenberg/proof_system/arithmetization/arithmetization.hpp"
#include "barretenberg/transcript/manifest.hpp"

namespace honk {
Expand Down Expand Up @@ -63,8 +64,9 @@ struct StandardArithmetization {
namespace honk {
struct StandardHonk {
public:
using Arithmetization = honk::StandardArithmetization;
using MULTIVARIATE = Arithmetization::POLYNOMIAL;
// This whole file is broken; changes here are in anticipation of a follow-up rework of the flavor specificaiton.
using Arithmetization = arithmetization::Standard;
using MULTIVARIATE = honk::StandardArithmetization::POLYNOMIAL;
// // TODO(Cody): Where to specify? is this polynomial manifest size?
// static constexpr size_t STANDARD_HONK_MANIFEST_SIZE = 16;
// TODO(Cody): Maybe relation should be supplied and this should be computed as is done in sumcheck?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class standard_verifier_settings : public plonk::standard_settings {
typedef transcript::StandardTranscript Transcript;
static constexpr size_t num_challenge_bytes = 16;
static constexpr transcript::HashType hash_type = transcript::HashType::PedersenBlake3s;
static constexpr size_t program_width = 3;
static constexpr size_t num_wires = honk::StandardHonk::Arithmetization::num_wires;
static constexpr size_t num_polys = honk::StandardArithmetization::NUM_POLYNOMIALS;
};

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/barretenberg/honk/proof_system/prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Prover<settings>::Prover(std::vector<barretenberg::polynomial>&& wire_polys,
* */
template <typename settings> void Prover<settings>::compute_wire_commitments()
{
for (size_t i = 0; i < settings::program_width; ++i) {
for (size_t i = 0; i < settings::Arithmetization::num_wires; ++i) {
auto commitment = commitment_key->commit(wire_polynomials[i]);

transcript.send_to_verifier("W_" + std::to_string(i + 1), commitment);
Expand Down
10 changes: 5 additions & 5 deletions cpp/src/barretenberg/honk/proof_system/verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ template <typename program_settings> bool Verifier<program_settings>::verify_pro
const size_t NUM_UNSHIFTED = honk::StandardArithmetization::NUM_UNSHIFTED_POLYNOMIALS;
const size_t NUM_PRECOMPUTED = honk::StandardArithmetization::NUM_PRECOMPUTED_POLYNOMIALS;

constexpr auto program_width = program_settings::program_width;
constexpr auto num_wires = program_settings::num_wires;

transcript = VerifierTranscript<FF>{ proof.proof_data };

Expand All @@ -109,8 +109,8 @@ template <typename program_settings> bool Verifier<program_settings>::verify_pro
}

// Get commitments to the wires
std::array<CommitmentAffine, program_width> wire_commitments;
for (size_t i = 0; i < program_width; ++i) {
std::array<CommitmentAffine, num_wires> wire_commitments;
for (size_t i = 0; i < num_wires; ++i) {
wire_commitments[i] = transcript.template receive_from_prover<CommitmentAffine>("W_" + std::to_string(i + 1));
}

Expand Down Expand Up @@ -169,11 +169,11 @@ template <typename program_settings> bool Verifier<program_settings>::verify_pro
batched_commitment_unshifted += commitment * rhos[i];
}
// add wire commitments
for (size_t i = 0; i < program_width; ++i) {
for (size_t i = 0; i < num_wires; ++i) {
batched_commitment_unshifted += wire_commitments[i] * rhos[NUM_PRECOMPUTED + i];
}
// add z_permutation commitment
batched_commitment_unshifted += z_permutation_commitment * rhos[NUM_PRECOMPUTED + program_width];
batched_commitment_unshifted += z_permutation_commitment * rhos[NUM_PRECOMPUTED + num_wires];

// Construct batched commitment for to-be-shifted polynomials
batched_commitment_to_be_shifted = z_permutation_commitment * rhos[NUM_UNSHIFTED];
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/barretenberg/plonk/composer/composer_base.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once
#include "barretenberg/ecc/curves/bn254/fr.hpp"
#include "barretenberg/proof_system/composer/composer_base.hpp"
#include "barretenberg/proof_system/arithmetization/gate_data.hpp"
#include "barretenberg/plonk/proof_system/prover/prover.hpp"
#include "barretenberg/plonk/proof_system/verifier/verifier.hpp"
#include "barretenberg/plonk/proof_system/types/prover_settings.hpp"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class StandardPlonkComposer {

// Leaving it in for now just in case
bool contains_recursive_proof = false;
static constexpr size_t program_width = STANDARD_WIDTH;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

program_width is more pervasive in plonk so I left that name in place there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Trying to minimize footprint, at least for now.

static constexpr size_t program_width = StandardCircuitConstructor::program_width;

/**Standard methods*/

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#pragma once
#include "barretenberg/proof_system/arithmetization/arithmetization.hpp"
#include "barretenberg/transcript/transcript.hpp"
namespace plonk {
class settings_base {
Expand All @@ -11,6 +12,7 @@ class settings_base {

class standard_settings : public settings_base {
public:
using Arithmetization = arithmetization::Standard;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a temporary measure, will refactor more fully with flavor work.

static constexpr size_t num_challenge_bytes = 16;
static constexpr transcript::HashType hash_type = transcript::HashType::PedersenBlake3s;
static constexpr size_t program_width = 3;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#pragma once
#include <cstddef>

namespace arithmetization {

template <size_t _num_wires, size_t _num_selectors> struct Arithmetization {
static constexpr size_t num_wires = _num_wires;
static constexpr size_t num_selectors = _num_selectors;
// Note: For even greater modularity, in each instantiation we could specify a list of components here, where a
// component is a meaningful collection of functions for creating gates, as in:
//
// struct Component {
// using Arithmetic = component::Arithmetic3Wires;
// using RangeConstraints = component::Base4Accumulators or component::GenPerm or...
// using LooupTables = component::Plookup4Wire or component::CQ8Wire or...
// ...
// };
//
// We should only do this if it becomes necessary or convenient.
};

// These are not magic numbers and they should not be written with global constants. These paraters are not accessible
// through clearly named static class members.
using Standard = Arithmetization<3, 5>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

To me a magic number is any number that appears without a name (and without explanation) in the code. Aside from being unexplained, they are also unsearchable. We should find a way to avoid global constants AND hard coded numbers.

Copy link
Collaborator Author

@codygunton codygunton Mar 31, 2023

Choose a reason for hiding this comment

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

But everywhere outside of this file this numbers should be accessed as arithmetization::Standard::num_wires and arithmetization::Standard::num_selectors. They are part of a class definition, and in the same file you see the names of these in the base class. I would really hate to have NUM_STANDARD_WIRES and also have that leak outside of this file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added argument comments to clarify as in
using Standard = Arithmetization</*num_wires =*/3, /*num_selectors =*/5>;

using Turbo = Arithmetization<4, 11>;

} // namespace arithmetization
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ namespace bonk {
* @param b_variable_idx Index of a variable in class b.
* @param msg Class tag.
* */
template <size_t program_width_>
void CircuitConstructorBase<program_width_>::assert_equal(const uint32_t a_variable_idx,
const uint32_t b_variable_idx,
std::string const& msg)
template <typename Arithmetization>
void CircuitConstructorBase<Arithmetization>::assert_equal(const uint32_t a_variable_idx,
const uint32_t b_variable_idx,
std::string const& msg)
{
assert_valid_variables({ a_variable_idx, b_variable_idx });
bool values_equal = (get_variable(a_variable_idx) == get_variable(b_variable_idx));
Expand Down Expand Up @@ -41,6 +41,6 @@ void CircuitConstructorBase<program_width_>::assert_equal(const uint32_t a_varia
real_variable_tags[a_real_idx] = real_variable_tags[b_real_idx];
}
// Standard honk/ plonk instantiation
template class CircuitConstructorBase<3>;
template class CircuitConstructorBase<4>;
template class CircuitConstructorBase<arithmetization::Standard>;
template class CircuitConstructorBase<arithmetization::Turbo>;
} // namespace bonk
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
#pragma once
#include "barretenberg/proof_system/composer/composer_base.hpp"
#include "barretenberg/proof_system/arithmetization/arithmetization.hpp"
#include "barretenberg/proof_system/arithmetization/gate_data.hpp"
#include "barretenberg/ecc/curves/bn254/fr.hpp"
#include <utility>

using namespace bonk;
namespace bonk {
static constexpr uint32_t DUMMY_TAG = 0;

template <size_t program_width_> class CircuitConstructorBase {
template <typename Arithmetization> class CircuitConstructorBase {
public:
static constexpr size_t program_width = program_width_;
static constexpr size_t num_wires = Arithmetization::num_wires;
// Keeping num_wires, at least temporarily, for backward compatibility
static constexpr size_t program_width = Arithmetization::num_wires;
static constexpr size_t num_selectors = Arithmetization::num_selectors;
// TODO(Cody): selector names are used by composer helper. They can therefore be specified through the proving
// system flavor. Getting rid of this also lets us get rid of the weird constructor that's uses the selector names
// functions
std::vector<std::string> selector_names_;
size_t num_gates = 0;
// TODO(#216)(Adrian): It would be better to store an array of size program_width_
// to make the composer agnostic of the wire name.
std::vector<uint32_t> w_l;
std::vector<uint32_t> w_r;
std::vector<uint32_t> w_o;
std::vector<uint32_t> w_4;

std::array<std::vector<uint32_t>, num_wires> wires;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice

std::array<std::vector<barretenberg::fr>, num_selectors> selectors;

std::vector<uint32_t> public_inputs;
std::vector<barretenberg::fr> variables;
// index of next variable in equivalence class (=REAL_VARIABLE if you're last)
Expand All @@ -33,8 +37,6 @@ template <size_t program_width_> class CircuitConstructorBase {
// DOCTODO(#231): replace with the relevant wiki link.
std::map<uint32_t, uint32_t> tau;

size_t num_selectors;
std::vector<std::vector<barretenberg::fr>> selectors;
numeric::random::Engine* rand_engine = nullptr;
bool _failed = false;
std::string _err;
Expand All @@ -46,10 +48,8 @@ template <size_t program_width_> class CircuitConstructorBase {
// Cody: This is used by compute_wire_copy_cycles in Plonk.
// enum WireType { LEFT = 0U, RIGHT = (1U << 30U), OUTPUT = (1U << 31U), FOURTH = 0xc0000000 };

CircuitConstructorBase(std::vector<std::string> selector_names, size_t num_selectors = 0, size_t size_hint = 0)
CircuitConstructorBase(std::vector<std::string> selector_names, size_t size_hint = 0)
: selector_names_(std::move(selector_names))
, num_selectors(num_selectors)
, selectors(num_selectors)
{
for (auto& p : selectors) {
p.reserve(size_hint);
Expand Down
Loading