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 3 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,44 @@ TEST(MegaCircuitBuilder, GoblinEccOpQueueUltraOps)
}
}

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);

// Check that for the blocks arithmetic, delta_range, elliptic, aux, lookup, busread, poseidon2_external,
// poseidon2_internal the corresponding selector polynomial is non-zero on the appropriate rows and the other
// selectors are zero get the trace blocks
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,77 @@ 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 each block only contains gates which have the corresponding selector turned on.
*
*/
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);

// Check that in the aux blocks, only the aux selector is nonzero, while the other selectors 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 +762,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 +792,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 +863,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 +898,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 @@ -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