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.

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

chore: Reenable some ultra honk composer tests #2417

Merged
merged 4 commits into from
Sep 25, 2023
Merged

chore: Reenable some ultra honk composer tests #2417

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb9d1d4
fix and reenable some honk ultra composer tests
ledwards2225 Sep 19, 2023
6c94f50
remove duplicated commented out code
ledwards2225 Sep 19, 2023
3fca291
Merge branch 'master' into lde/reenable_ultra_tests
ledwards2225 Sep 22, 2023
7d358e2
Merge branch 'master' into lde/reenable_ultra_tests
ledwards2225 Sep 25, 2023
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
147 changes: 63 additions & 84 deletions barretenberg/cpp/src/barretenberg/honk/composer/ultra_composer.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ void ensure_non_zero(auto& polynomial)
}

class UltraHonkComposerTests : public ::testing::Test {
public:
using fr = barretenberg::fr;

protected:
static void SetUpTestSuite() { barretenberg::srs::init_crs_factory("../srs_db/ignition"); }
};
Expand Down Expand Up @@ -91,7 +94,6 @@ TEST_F(UltraHonkComposerTests, ANonZeroPolynomialIsAGoodPolynomial)
*/
TEST_F(UltraHonkComposerTests, PublicInputs)
{
using fr = barretenberg::fr;
auto builder = proof_system::UltraCircuitBuilder();
size_t num_gates = 10;

Expand All @@ -116,7 +118,6 @@ TEST_F(UltraHonkComposerTests, PublicInputs)

TEST_F(UltraHonkComposerTests, XorConstraint)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();

uint32_t left_value = engine.get_random_uint32();
Expand Down Expand Up @@ -145,7 +146,6 @@ TEST_F(UltraHonkComposerTests, XorConstraint)

TEST_F(UltraHonkComposerTests, create_gates_from_plookup_accumulators)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();

barretenberg::fr input_value = fr::random_element();
Expand Down Expand Up @@ -241,7 +241,6 @@ TEST_F(UltraHonkComposerTests, create_gates_from_plookup_accumulators)

TEST_F(UltraHonkComposerTests, test_no_lookup_proof)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();

for (size_t i = 0; i < 16; ++i) {
Expand Down Expand Up @@ -303,7 +302,6 @@ TEST_F(UltraHonkComposerTests, test_elliptic_gate)

TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
fr a = fr::random_element();
fr b = -a;
Expand Down Expand Up @@ -332,7 +330,6 @@ TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation)

TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation_and_cycles)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
fr a = fr::random_element();
fr c = -a;
Expand Down Expand Up @@ -371,7 +368,6 @@ TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation_and_cycles)

TEST_F(UltraHonkComposerTests, bad_tag_permutation)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
fr a = fr::random_element();
fr b = -a;
Expand Down Expand Up @@ -399,7 +395,6 @@ TEST_F(UltraHonkComposerTests, bad_tag_permutation)
// same as above but with turbocomposer to check reason of failue is really tag mismatch
TEST_F(UltraHonkComposerTests, bad_tag_turbo_permutation)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
fr a = fr::random_element();
fr b = -a;
Expand All @@ -425,7 +420,6 @@ TEST_F(UltraHonkComposerTests, bad_tag_turbo_permutation)

TEST_F(UltraHonkComposerTests, sort_widget)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
fr a = fr::one();
fr b = fr(2);
Expand All @@ -444,8 +438,6 @@ TEST_F(UltraHonkComposerTests, sort_widget)

TEST_F(UltraHonkComposerTests, sort_with_edges_gate)
{
using fr = barretenberg::fr;

fr a = fr::one();
fr b = fr(2);
fr c = fr(3);
Expand Down Expand Up @@ -617,7 +609,6 @@ TEST_F(UltraHonkComposerTests, range_constraint)

TEST_F(UltraHonkComposerTests, range_with_gates)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
auto idx = add_variables(circuit_builder, { 1, 2, 3, 4, 5, 6, 7, 8 });
for (size_t i = 0; i < idx.size(); i++) {
Expand All @@ -637,7 +628,6 @@ TEST_F(UltraHonkComposerTests, range_with_gates)

TEST_F(UltraHonkComposerTests, range_with_gates_where_range_is_not_a_power_of_two)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
auto idx = add_variables(circuit_builder, { 1, 2, 3, 4, 5, 6, 7, 8 });
for (size_t i = 0; i < idx.size(); i++) {
Expand All @@ -657,7 +647,6 @@ TEST_F(UltraHonkComposerTests, range_with_gates_where_range_is_not_a_power_of_tw

TEST_F(UltraHonkComposerTests, sort_widget_complex)
{
using fr = barretenberg::fr;
{

auto circuit_builder = proof_system::UltraCircuitBuilder();
Expand Down Expand Up @@ -686,7 +675,6 @@ TEST_F(UltraHonkComposerTests, sort_widget_complex)

TEST_F(UltraHonkComposerTests, sort_widget_neg)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
fr a = fr::one();
fr b = fr(2);
Expand All @@ -705,7 +693,6 @@ TEST_F(UltraHonkComposerTests, sort_widget_neg)

TEST_F(UltraHonkComposerTests, composed_range_constraint)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();
auto c = fr::random_element();
auto d = uint256_t(c).slice(0, 133);
Expand All @@ -720,7 +707,6 @@ TEST_F(UltraHonkComposerTests, composed_range_constraint)

TEST_F(UltraHonkComposerTests, non_native_field_multiplication)
{
using fr = barretenberg::fr;
using fq = barretenberg::fq;
auto circuit_builder = proof_system::UltraCircuitBuilder();

Expand Down Expand Up @@ -778,7 +764,6 @@ TEST_F(UltraHonkComposerTests, non_native_field_multiplication)

TEST_F(UltraHonkComposerTests, rom)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();

uint32_t rom_values[8]{
Expand Down Expand Up @@ -821,7 +806,6 @@ TEST_F(UltraHonkComposerTests, rom)

TEST_F(UltraHonkComposerTests, ram)
{
using fr = barretenberg::fr;
auto circuit_builder = proof_system::UltraCircuitBuilder();

uint32_t ram_values[8]{
Expand Down Expand Up @@ -884,6 +868,66 @@ TEST_F(UltraHonkComposerTests, ram)
prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
}

TEST_F(UltraHonkComposerTests, range_checks_on_duplicates)
Copy link
Contributor

Choose a reason for hiding this comment

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

is the only difference that the circuit_builder is defined in the new test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah there's no meaningful change, I'm just brining this outdated test up to the modern standard. The logic of what we now call a circuit builder used to be contained in the "composers". The logic is otherwise the same.

{
auto circuit_builder = proof_system::UltraCircuitBuilder();

uint32_t a = circuit_builder.add_variable(100);
uint32_t b = circuit_builder.add_variable(100);
uint32_t c = circuit_builder.add_variable(100);
uint32_t d = circuit_builder.add_variable(100);

circuit_builder.assert_equal(a, b);
circuit_builder.assert_equal(a, c);
circuit_builder.assert_equal(a, d);

circuit_builder.create_new_range_constraint(a, 1000);
circuit_builder.create_new_range_constraint(b, 1001);
circuit_builder.create_new_range_constraint(c, 999);
circuit_builder.create_new_range_constraint(d, 1000);

circuit_builder.create_big_add_gate(
{
a,
b,
c,
d,
0,
0,
0,
0,
0,
},
false);

auto composer = UltraComposer();
prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
}

// Ensure copy constraints added on variables smaller than 2^14, which have been previously
// range constrained, do not break the set equivalence checks because of indices mismatch.
// 2^14 is DEFAULT_PLOOKUP_RANGE_BITNUM i.e. the maximum size before a variable gets sliced
// before range constraints are applied to it.
TEST_F(UltraHonkComposerTests, range_constraint_small_variable)
{
auto circuit_builder = proof_system::UltraCircuitBuilder();

uint16_t mask = (1 << 8) - 1;
int a = engine.get_random_uint16() & mask;
uint32_t a_idx = circuit_builder.add_variable(fr(a));
uint32_t b_idx = circuit_builder.add_variable(fr(a));
ASSERT_NE(a_idx, b_idx);
uint32_t c_idx = circuit_builder.add_variable(fr(a));
ASSERT_NE(c_idx, b_idx);
circuit_builder.create_range_constraint(b_idx, 8, "bad range");
Copy link
Contributor

Choose a reason for hiding this comment

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

is this 8 arbitrary or does it have meaning related to range constraints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it's arbitrary other than 8 < 14 per the comment. I don't have full context here but I think the point is simply to check that range constraining a value (small enough to require only 1 range constraint) prior to copy constraining it to another value yields a valid proof. Apparently previously there was a bug that caused such an operation to fail.

circuit_builder.assert_equal(a_idx, b_idx);
circuit_builder.create_range_constraint(c_idx, 8, "bad range");
circuit_builder.assert_equal(a_idx, c_idx);

auto composer = UltraComposer();
prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
}

TEST(UltraGrumpkinHonkComposer, XorConstraint)
{
using fr = barretenberg::fr;
Expand Down Expand Up @@ -915,69 +959,4 @@ TEST(UltraGrumpkinHonkComposer, XorConstraint)
auto composer = UltraGrumpkinComposer();
prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
}

// TODO(#378)(luke): this is a recent update from Zac and fails; do we need a corresponding bug fix in ultra circuit
// c_Fonstructor? TEST(UltraHonkComposerTests, range_checks_on_duplicates)
// {
// auto composer = UltraComposer();

// uint32_t a = circuit_builder.add_variable(100);
// uint32_t b = circuit_builder.add_variable(100);
// uint32_t c = circuit_builder.add_variable(100);
// uint32_t d = circuit_builder.add_variable(100);

// circuit_builder.assert_equal(a, b);
// circuit_builder.assert_equal(a, c);
// circuit_builder.assert_equal(a, d);

// circuit_builder.create_new_range_constraint(a, 1000);
// circuit_builder.create_new_range_constraint(b, 1001);
// circuit_builder.create_new_range_constraint(c, 999);
// circuit_builder.create_new_range_constraint(d, 1000);

// circuit_builder.create_big_add_gate(
// {
// a,
// b,
// c,
// d,
// 0,
// 0,
// 0,
// 0,
// 0,
// },
// false);

// prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
// }

// TODO(#378)(luke): this is a new test from Zac; ultra circuit constructor does not yet have create_range_constraint
// implemented.
// // Ensure copy constraints added on variables smaller than 2^14, which have been previously
// // range constrained, do not break the set equivalence checks because of indices mismatch.
// // 2^14 is DEFAULT_PLOOKUP_RANGE_BITNUM i.e. the maximum size before a variable gets sliced
// // before range constraints are applied to it.
// T_FEST(UltraHonkComposerTests, range_constraint_small_variable)
// {
// auto composer = UltraComposer();
// uint16_t mask = (1 << 8) - 1;
// int a = engine.get_random_uint16() & mask;
// uint32_t a_idx = circuit_builder.add_variable(fr(a));
// uint32_t b_idx = circuit_builder.add_variable(fr(a));
// ASSERT_NE(a_idx, b_idx);
// uint32_t c_idx = circuit_builder.add_variable(fr(a));
// ASSERT_NE(c_idx, b_idx);
// composer.create_range_constraint(b_idx, 8, "bad range");
// circuit_builder.assert_equal(a_idx, b_idx);
// composer.create_range_constraint(c_idx, 8, "bad range");
// circuit_builder.assert_equal(a_idx, c_idx);

// auto prover = composer.create_prover(circuit_builder);
// auto proof = prover.construct_proof();
// auto verifier = composer.create_verifier(circuit_builder);
// bool result = verifier.verify_proof(proof);
// EXPECT_EQ(result, true);
// }

} // namespace test_ultra_honk_composer
Loading