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

chore: Moves add gate out of aux #8541

Merged
merged 8 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp"
#include "barretenberg/circuit_checker/circuit_checker.hpp"
#include "barretenberg/goblin/mock_circuits.hpp"
#include "barretenberg/stdlib/primitives/bigfield/constants.hpp"
#include <gtest/gtest.h>

Expand Down Expand Up @@ -158,4 +159,48 @@ TEST(MegaCircuitBuilder, GoblinEccOpQueueUltraOps)
}
}

/**
* @brief Check that the selector partitioning is correct for the mega circuit builder
* @details We check that for the arithmetic, delta_range, elliptic, aux, lookup, busread, poseidon2_external,
* poseidon2_internal blocks, and the other selectors are zero on that block.
*/
TEST(MegaCircuitBuilder, CompleteSelectorPartitioningCheck)
lucasxia01 marked this conversation as resolved.
Show resolved Hide resolved
{
auto builder = MegaCircuitBuilder();
GoblinMockCircuits::construct_simple_circuit(builder);
Copy link
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 this is the best circuit to use, but it seemed pretty diverse

bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, true);

// For each block, we want to check that all of the other selectors are zero on that block besides the one
// corresponding to the current block
for (auto& block : builder.blocks.get()) {
for (size_t i = 0; i < block.size(); ++i) {
if (&block != &builder.blocks.arithmetic) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using the ptr is a little weird, but it works. Could change to using an enum or something instead

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems fine

EXPECT_EQ(block.q_arith()[i], 0);
}
if (&block != &builder.blocks.delta_range) {
EXPECT_EQ(block.q_delta_range()[i], 0);
}
if (&block != &builder.blocks.elliptic) {
EXPECT_EQ(block.q_elliptic()[i], 0);
}
if (&block != &builder.blocks.aux) {
EXPECT_EQ(block.q_aux()[i], 0);
}
if (&block != &builder.blocks.lookup) {
EXPECT_EQ(block.q_lookup_type()[i], 0);
}
if (&block != &builder.blocks.busread) {
EXPECT_EQ(block.q_busread()[i], 0);
}
if (&block != &builder.blocks.poseidon2_external) {
EXPECT_EQ(block.q_poseidon2_external()[i], 0);
}
if (&block != &builder.blocks.poseidon2_internal) {
EXPECT_EQ(block.q_poseidon2_internal()[i], 0);
}
}
}
}

} // namespace bb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace bb {
using plookup::ColumnIdx;
using plookup::MultiTableId;

TEST(ultra_circuit_constructor, copy_constructor)
TEST(UltraCircuitConstructor, CopyConstructor)
Copy link
Contributor Author

@lucasxia01 lucasxia01 Sep 13, 2024

Choose a reason for hiding this comment

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

was testing out ai editor to do these formatting changes

{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();

Expand Down Expand Up @@ -43,7 +43,7 @@ TEST(ultra_circuit_constructor, copy_constructor)
EXPECT_TRUE(CircuitChecker::check(duplicate_circuit_constructor));
}

TEST(ultra_circuit_constructor, create_gates_from_plookup_accumulators)
TEST(UltraCircuitConstructor, CreateGatesFromPlookupAccumulators)
{

UltraCircuitBuilder circuit_builder = UltraCircuitBuilder();
Expand Down Expand Up @@ -105,7 +105,7 @@ TEST(ultra_circuit_constructor, create_gates_from_plookup_accumulators)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, bad_lookup_failure)
TEST(UltraCircuitConstructor, BadLookupFailure)
{
UltraCircuitBuilder builder;
MockCircuits::add_lookup_gates(builder);
Expand All @@ -121,15 +121,15 @@ TEST(ultra_circuit_constructor, bad_lookup_failure)
EXPECT_FALSE(CircuitChecker::check(builder));
}

TEST(ultra_circuit_constructor, base_case)
TEST(UltraCircuitConstructor, BaseCase)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
fr a = fr::one();
circuit_constructor.add_public_variable(a);
bool result = CircuitChecker::check(circuit_constructor);
EXPECT_EQ(result, true);
}
TEST(ultra_circuit_constructor, test_no_lookup_proof)
TEST(UltraCircuitConstructor, TestNoLookupProof)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();

Expand All @@ -152,7 +152,7 @@ TEST(ultra_circuit_constructor, test_no_lookup_proof)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, test_elliptic_gate)
TEST(UltraCircuitConstructor, TestEllipticGate)
{
typedef grumpkin::g1::affine_element affine_element;
typedef grumpkin::g1::element element;
Expand Down Expand Up @@ -180,7 +180,7 @@ TEST(ultra_circuit_constructor, test_elliptic_gate)
EXPECT_EQ(CircuitChecker::check(circuit_constructor), false);
}

TEST(ultra_circuit_constructor, test_elliptic_double_gate)
TEST(UltraCircuitConstructor, TestEllipticDoubleGate)
{
typedef grumpkin::g1::affine_element affine_element;
typedef grumpkin::g1::element element;
Expand All @@ -200,7 +200,7 @@ TEST(ultra_circuit_constructor, test_elliptic_double_gate)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, non_trivial_tag_permutation)
TEST(UltraCircuitConstructor, NonTrivialTagPermutation)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
fr a = fr::random_element();
Expand Down Expand Up @@ -232,7 +232,7 @@ TEST(ultra_circuit_constructor, non_trivial_tag_permutation)
EXPECT_EQ(CircuitChecker::check(circuit_constructor), false);
}

TEST(ultra_circuit_constructor, non_trivial_tag_permutation_and_cycles)
TEST(UltraCircuitConstructor, NonTrivialTagPermutationAndCycles)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
fr a = fr::random_element();
Expand Down Expand Up @@ -273,7 +273,7 @@ TEST(ultra_circuit_constructor, non_trivial_tag_permutation_and_cycles)
circuit_constructor.real_variable_tags[circuit_constructor.real_variable_index[a_idx]] = 2;
EXPECT_EQ(CircuitChecker::check(circuit_constructor), false);
}
TEST(ultra_circuit_constructor, bad_tag_permutation)
TEST(UltraCircuitConstructor, BadTagPermutation)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
fr a = fr::random_element();
Expand Down Expand Up @@ -302,7 +302,7 @@ TEST(ultra_circuit_constructor, bad_tag_permutation)
EXPECT_EQ(result, false);
}

TEST(ultra_circuit_constructor, sort_widget)
TEST(UltraCircuitConstructor, SortWidget)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
fr a = fr::one();
Expand All @@ -328,7 +328,7 @@ std::vector<uint32_t> add_variables(UltraCircuitBuilder& circuit_constructor, st
}
return res;
}
TEST(ultra_circuit_constructor, sort_with_edges_gate)
TEST(UltraCircuitConstructor, SortWithEdgesGate)
{
fr a = fr::one();
fr b = fr(2);
Expand Down Expand Up @@ -421,7 +421,7 @@ TEST(ultra_circuit_constructor, sort_with_edges_gate)
}
}

TEST(ultra_circuit_constructor, range_constraint)
TEST(UltraCircuitConstructor, RangeConstraint)
{
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
Expand Down Expand Up @@ -490,7 +490,7 @@ TEST(ultra_circuit_constructor, range_constraint)
}
}

TEST(ultra_circuit_constructor, range_with_gates)
TEST(UltraCircuitConstructor, RangeWithGates)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
auto idx = add_variables(circuit_constructor, { 1, 2, 3, 4, 5, 6, 7, 8 });
Expand All @@ -510,7 +510,7 @@ TEST(ultra_circuit_constructor, range_with_gates)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, range_with_gates_where_range_is_not_a_power_of_two)
TEST(UltraCircuitConstructor, RangeWithGatesWhereRangeIsNotAPowerOfTwo)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
auto idx = add_variables(circuit_constructor, { 1, 2, 3, 4, 5, 6, 7, 8 });
Expand All @@ -530,7 +530,7 @@ TEST(ultra_circuit_constructor, range_with_gates_where_range_is_not_a_power_of_t
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, sort_widget_complex)
TEST(UltraCircuitConstructor, SortWidgetComplex)
{
{

Expand All @@ -557,7 +557,7 @@ TEST(ultra_circuit_constructor, sort_widget_complex)
EXPECT_EQ(result, false);
}
}
TEST(ultra_circuit_constructor, sort_widget_neg)
TEST(UltraCircuitConstructor, SortWidgetNeg)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
fr a = fr::one();
Expand All @@ -575,7 +575,7 @@ TEST(ultra_circuit_constructor, sort_widget_neg)
EXPECT_EQ(result, false);
}

TEST(ultra_circuit_constructor, composed_range_constraint)
TEST(UltraCircuitConstructor, ComposedRangeConstraint)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
auto c = fr::random_element();
Expand All @@ -590,7 +590,7 @@ TEST(ultra_circuit_constructor, composed_range_constraint)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, non_native_field_multiplication)
TEST(UltraCircuitConstructor, NonNativeFieldMultiplication)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();

Expand Down Expand Up @@ -646,7 +646,78 @@ TEST(ultra_circuit_constructor, non_native_field_multiplication)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, rom)
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test copies the previous test and uses the function that previously, but now doesn't, added a gate with q_arith turned on in the aux block. It checks that the aux block does not have any other selector turned on besides the aux one

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to have this, but you're only checking one block.

* @brief Test that the aux block only contains aux gates.
*
*/
TEST(UltraCircuitConstructor, NonNativeFieldMultiplicationSortCheck)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();

fq a = fq::random_element();
fq b = fq::random_element();
uint256_t modulus = fq::modulus;

uint1024_t a_big = uint512_t(uint256_t(a));
uint1024_t b_big = uint512_t(uint256_t(b));
uint1024_t p_big = uint512_t(uint256_t(modulus));

uint1024_t q_big = (a_big * b_big) / p_big;
uint1024_t r_big = (a_big * b_big) % p_big;

uint256_t q(q_big.lo.lo);
uint256_t r(r_big.lo.lo);

const auto split_into_limbs = [&](const uint512_t& input) {
constexpr size_t NUM_BITS = 68;
std::array<fr, 5> limbs;
limbs[0] = input.slice(0, NUM_BITS).lo;
limbs[1] = input.slice(NUM_BITS * 1, NUM_BITS * 2).lo;
limbs[2] = input.slice(NUM_BITS * 2, NUM_BITS * 3).lo;
limbs[3] = input.slice(NUM_BITS * 3, NUM_BITS * 4).lo;
limbs[4] = fr(input.lo);
return limbs;
};

const auto get_limb_witness_indices = [&](const std::array<fr, 5>& limbs) {
std::array<uint32_t, 5> limb_indices;
limb_indices[0] = circuit_constructor.add_variable(limbs[0]);
limb_indices[1] = circuit_constructor.add_variable(limbs[1]);
limb_indices[2] = circuit_constructor.add_variable(limbs[2]);
limb_indices[3] = circuit_constructor.add_variable(limbs[3]);
limb_indices[4] = circuit_constructor.add_variable(limbs[4]);
return limb_indices;
};
const uint512_t BINARY_BASIS_MODULUS = uint512_t(1) << (68 * 4);
auto modulus_limbs = split_into_limbs(BINARY_BASIS_MODULUS - uint512_t(modulus));

const auto a_indices = get_limb_witness_indices(split_into_limbs(uint256_t(a)));
const auto b_indices = get_limb_witness_indices(split_into_limbs(uint256_t(b)));
const auto q_indices = get_limb_witness_indices(split_into_limbs(uint256_t(q)));
const auto r_indices = get_limb_witness_indices(split_into_limbs(uint256_t(r)));

non_native_field_witnesses<fr> inputs{
a_indices, b_indices, q_indices, r_indices, modulus_limbs, fr(uint256_t(modulus)),
};
const auto [lo_1_idx, hi_1_idx] = circuit_constructor.evaluate_non_native_field_multiplication(inputs);
circuit_constructor.range_constrain_two_limbs(lo_1_idx, hi_1_idx, 70, 70);

bool result = CircuitChecker::check(circuit_constructor);
EXPECT_EQ(result, true);

// Everything above was copied from the previous test.
// Check that in the aux blocks, the other selectors besides the aux selector are zero
for (size_t i = 0; i < circuit_constructor.blocks.aux.size(); ++i) {
EXPECT_EQ(circuit_constructor.blocks.aux.q_arith()[i], 0);
EXPECT_EQ(circuit_constructor.blocks.aux.q_delta_range()[i], 0);
EXPECT_EQ(circuit_constructor.blocks.aux.q_elliptic()[i], 0);
EXPECT_EQ(circuit_constructor.blocks.aux.q_lookup_type()[i], 0);
EXPECT_EQ(circuit_constructor.blocks.aux.q_poseidon2_external()[i], 0);
EXPECT_EQ(circuit_constructor.blocks.aux.q_poseidon2_internal()[i], 0);
}
}

TEST(UltraCircuitConstructor, Rom)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();

Expand Down Expand Up @@ -692,7 +763,7 @@ TEST(ultra_circuit_constructor, rom)
* @brief A simple-as-possible RAM read test, for easier debugging
*
*/
TEST(ultra_circuit_constructor, ram_simple)
TEST(UltraCircuitConstructor, RamSimple)
{
UltraCircuitBuilder builder;

Expand Down Expand Up @@ -722,7 +793,7 @@ TEST(ultra_circuit_constructor, ram_simple)
EXPECT_TRUE(CircuitChecker::check(builder));
}

TEST(ultra_circuit_constructor, ram)
TEST(UltraCircuitConstructor, Ram)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();

Expand Down Expand Up @@ -793,7 +864,7 @@ TEST(ultra_circuit_constructor, ram)
EXPECT_TRUE(CircuitChecker::check(duplicate_circuit_constructor));
}

TEST(ultra_circuit_constructor, range_checks_on_duplicates)
TEST(UltraCircuitConstructor, RangeChecksOnDuplicates)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();

Expand Down Expand Up @@ -828,7 +899,7 @@ TEST(ultra_circuit_constructor, range_checks_on_duplicates)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, check_circuit_showcase)
TEST(UltraCircuitConstructor, CheckCircuitShowcase)
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
// check_circuit allows us to check correctness on the go
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,39 @@ TEST_F(MegaMockCircuitsPinning, AppCircuitSizes)
};
run_test(true);
run_test(false);
}

/**
* @brief Regression test that the structured circuit size has not increased over a power of 2.
*/
TEST_F(MegaMockCircuitsPinning, SmallTestStructuredCircuitSize)
{
GoblinProver goblin;
MegaCircuitBuilder app_circuit{ goblin.op_queue };
auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, TraceStructure::SMALL_TEST);
EXPECT_EQ(proving_key->proving_key.log_circuit_size, 18);
}

TEST_F(MegaMockCircuitsPinning, ClientIVCBenchStructuredCircuitSize)
{
GoblinProver goblin;
MegaCircuitBuilder app_circuit{ goblin.op_queue };
auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, TraceStructure::CLIENT_IVC_BENCH);
EXPECT_EQ(proving_key->proving_key.log_circuit_size, 18);
}

TEST_F(MegaMockCircuitsPinning, AztecIVCBenchStructuredCircuitSize)
{
GoblinProver goblin;
MegaCircuitBuilder app_circuit{ goblin.op_queue };
auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, TraceStructure::AZTEC_IVC_BENCH);
EXPECT_EQ(proving_key->proving_key.log_circuit_size, 19);
}

TEST_F(MegaMockCircuitsPinning, E2EStructuredCircuitSize)
{
GoblinProver goblin;
MegaCircuitBuilder app_circuit{ goblin.op_queue };
auto proving_key = std::make_shared<DeciderProvingKey>(app_circuit, TraceStructure::E2E_FULL_TEST);
EXPECT_EQ(proving_key->proving_key.log_circuit_size, 20);
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ struct MaxBlockSizeTracker {
}

// For printing only. Must match the order of the members in the arithmetization
std::vector<std::string> block_labels{ "ecc_op", "pub_inputs", "arithmetic",
"delta_range", "elliptic", "aux",
"lookup", "busread", "poseidon_external",
"poseidon_internal" };
std::vector<std::string> block_labels{
"ecc_op", "pub_inputs", "arithmetic", "delta_range", "elliptic",
"aux", "lookup", "busread", "poseidon2_external", "poseidon2_internal"
};

void print()
{
Expand Down
Loading
Loading