-
Notifications
You must be signed in to change notification settings - Fork 592
feat: Add public data read gadget #13138
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
Changes from 11 commits
6157356
55a8ddd
169acb1
4484452
301f3fc
85a891f
951fa2a
6c6d0de
b9dca21
765a697
b78da93
a2ab083
f1802f6
659e77f
f2a5bf5
4798808
46cff64
24bce68
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,81 @@ | ||||||
| include "./merkle_check.pil"; | ||||||
| include "./ff_gt.pil"; | ||||||
| include "./poseidon2_hash.pil"; | ||||||
| include "./constants_gen.pil"; | ||||||
| include "./precomputed.pil"; | ||||||
|
|
||||||
| namespace public_data_read; | ||||||
| pol commit sel; | ||||||
| sel * (1 - sel) = 0; | ||||||
|
|
||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sirasistant We certainly can add a skippable condition.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yes pleaaase! I didn't pay attention but basically we shouldn't merge any gadget without skippable
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ugh, my bad, forgot about the skippable condition |
||||||
| // Inputs to the gadget | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usage of this gagdet is not clear at first sight. |
||||||
| pol commit value; | ||||||
| pol commit slot; | ||||||
| pol commit root; | ||||||
|
|
||||||
| // Hints | ||||||
| pol commit low_leaf_slot; | ||||||
| pol commit low_leaf_value; | ||||||
| pol commit low_leaf_next_index; | ||||||
| pol commit low_leaf_next_slot; | ||||||
|
|
||||||
| pol commit low_leaf_index; | ||||||
|
|
||||||
| // ========= LOW LEAF MEMBERSHIP ========= | ||||||
| pol commit low_leaf_hash; | ||||||
| // TODO: We need this temporarily while we dont allow for aliases in the lookup tuple | ||||||
|
||||||
| // TODO: We need this temporarily while we dont allow for aliases in the lookup tuple | |
| // TODO: We need this temporarily while we do not allow for aliases in the lookup tuple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment like: SLOT_LOW_LEAF_SLOT_DIFF == 0 <==> LEAF_EXISTS == 0
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // If doesn't exist, we need to validate that the slot is greater than the low leaf slot | |
| // If it doesn't exist, we need to validate that the slot is greater than the low leaf slot |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // TODO: We need this temporarily while we dont allow for aliases in the lookup tuple | |
| // TODO: We need this temporarily while we do not allow for aliases in the lookup tuple |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For readability, I would prefer to have this relation just after the definition of LEAF_EXISTS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one?
#[VALUE_IS_CORRECT]
low_leaf_value * LEAF_EXISTS - value = 0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,9 +70,9 @@ std::vector<TestParams> comparison_tests = { | |
| TestParams{ 0, -1, false } | ||
| }; | ||
|
|
||
| class BasicTest : public TestWithParam<TestParams> {}; | ||
| class FieldGreaterThanBasicTest : public TestWithParam<TestParams> {}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we were actually calling them SomethingConstrainingTest?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes but this class name is appended to INSTANTIATE_TEST_SUITE_P(FieldGreaterThanConstrainingTest,
FieldGreaterThanBasicTest,
::testing::ValuesIn(comparison_tests));results in something like this |
||
|
|
||
| TEST_P(BasicTest, BasicComparison) | ||
| TEST_P(FieldGreaterThanBasicTest, BasicComparison) | ||
| { | ||
| const auto& param = GetParam(); | ||
|
|
||
|
|
@@ -92,11 +92,13 @@ TEST_P(BasicTest, BasicComparison) | |
| check_relation<ff_gt>(trace); | ||
| } | ||
|
|
||
| INSTANTIATE_TEST_SUITE_P(FieldGreaterThanConstrainingTest, BasicTest, ::testing::ValuesIn(comparison_tests)); | ||
| INSTANTIATE_TEST_SUITE_P(FieldGreaterThanConstrainingTest, | ||
| FieldGreaterThanBasicTest, | ||
| ::testing::ValuesIn(comparison_tests)); | ||
|
|
||
| class InteractionsTests : public TestWithParam<TestParams> {}; | ||
| class FieldGreaterThanInteractionsTests : public TestWithParam<TestParams> {}; | ||
|
|
||
| TEST_P(InteractionsTests, InteractionsWithRangeCheck) | ||
| TEST_P(FieldGreaterThanInteractionsTests, InteractionsWithRangeCheck) | ||
| { | ||
| const auto& param = GetParam(); | ||
|
|
||
|
|
@@ -125,7 +127,9 @@ TEST_P(InteractionsTests, InteractionsWithRangeCheck) | |
| check_interaction<lookup_a_lo_range>(trace); | ||
| } | ||
|
|
||
| INSTANTIATE_TEST_SUITE_P(FieldGreaterThanConstrainingTest, InteractionsTests, ::testing::ValuesIn(comparison_tests)); | ||
| INSTANTIATE_TEST_SUITE_P(FieldGreaterThanConstrainingTest, | ||
| FieldGreaterThanInteractionsTests, | ||
| ::testing::ValuesIn(comparison_tests)); | ||
|
|
||
| TEST(FieldGreaterThanConstrainingTest, NegativeManipulatedDecompositions) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,236 @@ | ||
| #include <gmock/gmock.h> | ||
| #include <gtest/gtest.h> | ||
|
|
||
| #include <cmath> | ||
| #include <cstdint> | ||
|
|
||
| #include "barretenberg/crypto/poseidon2/poseidon2.hpp" | ||
| #include "barretenberg/vm2/constraining/flavor_settings.hpp" | ||
| #include "barretenberg/vm2/constraining/testing/check_relation.hpp" | ||
| #include "barretenberg/vm2/generated/relations/merkle_check.hpp" | ||
| #include "barretenberg/vm2/generated/relations/public_data_read.hpp" | ||
| #include "barretenberg/vm2/simulation/events/event_emitter.hpp" | ||
| #include "barretenberg/vm2/simulation/events/public_data_tree_read_event.hpp" | ||
| #include "barretenberg/vm2/simulation/field_gt.hpp" | ||
| #include "barretenberg/vm2/simulation/lib/merkle.hpp" | ||
| #include "barretenberg/vm2/simulation/poseidon2.hpp" | ||
| #include "barretenberg/vm2/simulation/public_data_tree_check.hpp" | ||
| #include "barretenberg/vm2/simulation/testing/mock_range_check.hpp" | ||
| #include "barretenberg/vm2/testing/fixtures.hpp" | ||
| #include "barretenberg/vm2/testing/macros.hpp" | ||
| #include "barretenberg/vm2/tracegen/field_gt_trace.hpp" | ||
| #include "barretenberg/vm2/tracegen/lib/lookup_builder.hpp" | ||
| #include "barretenberg/vm2/tracegen/merkle_check_trace.hpp" | ||
| #include "barretenberg/vm2/tracegen/poseidon2_trace.hpp" | ||
| #include "barretenberg/vm2/tracegen/public_data_tree_read_trace.hpp" | ||
| #include "barretenberg/vm2/tracegen/test_trace_container.hpp" | ||
|
|
||
| namespace bb::avm2::constraining { | ||
| namespace { | ||
|
|
||
| using ::testing::NiceMock; | ||
| using ::testing::TestWithParam; | ||
|
|
||
| using simulation::EventEmitter; | ||
| using simulation::FieldGreaterThan; | ||
| using simulation::FieldGreaterThanEvent; | ||
| using simulation::MerkleCheck; | ||
| using simulation::MerkleCheckEvent; | ||
| using simulation::MockRangeCheck; | ||
| using simulation::NoopEventEmitter; | ||
| using simulation::Poseidon2; | ||
| using simulation::Poseidon2HashEvent; | ||
| using simulation::Poseidon2PermutationEvent; | ||
| using simulation::PublicDataTreeCheck; | ||
| using simulation::PublicDataTreeLeafPreimage; | ||
| using simulation::PublicDataTreeReadEvent; | ||
| using simulation::root_from_path; | ||
|
|
||
| using tracegen::FieldGreaterThanTraceBuilder; | ||
| using tracegen::MerkleCheckTraceBuilder; | ||
| using tracegen::Poseidon2TraceBuilder; | ||
| using tracegen::PublicDataTreeReadTraceBuilder; | ||
| using tracegen::TestTraceContainer; | ||
|
|
||
| using FF = AvmFlavorSettings::FF; | ||
| using C = Column; | ||
| using public_data_read = bb::avm2::public_data_read<FF>; | ||
| using poseidon2 = crypto::Poseidon2<crypto::Poseidon2Bn254ScalarFieldParams>; | ||
| using PublicDataLeafValue = crypto::merkle_tree::PublicDataLeafValue; | ||
|
|
||
| TEST(PublicDataTreeReadConstrainingTest, EmptyRow) | ||
| { | ||
| check_relation<public_data_read>(testing::empty_trace()); | ||
| } | ||
|
|
||
| struct TestParams { | ||
| FF leaf_slot; | ||
| FF value; | ||
| PublicDataTreeLeafPreimage low_leaf; | ||
| }; | ||
|
|
||
| std::vector<TestParams> positive_tests = { | ||
| // Exists = true, leaf pointers to infinity | ||
| TestParams{ | ||
| .leaf_slot = 42, .value = 27, .low_leaf = PublicDataTreeLeafPreimage(PublicDataLeafValue(42, 27), 0, 0) }, | ||
| // Exists = true, leaf points to higher value | ||
| TestParams{ | ||
| .leaf_slot = 42, .value = 27, .low_leaf = PublicDataTreeLeafPreimage(PublicDataLeafValue(42, 27), 28, 50) }, | ||
| // Exists = false, low leaf points to infinity | ||
| TestParams{ .leaf_slot = 42, .value = 0, .low_leaf = PublicDataTreeLeafPreimage(PublicDataLeafValue(10, 0), 0, 0) }, | ||
| // Exists = false, low leaf points to higher value | ||
| TestParams{ | ||
| .leaf_slot = 42, .value = 0, .low_leaf = PublicDataTreeLeafPreimage(PublicDataLeafValue(10, 0), 28, 50) } | ||
| }; | ||
|
|
||
| class PublicDataReadPositiveTests : public TestWithParam<TestParams> {}; | ||
|
|
||
| TEST_P(PublicDataReadPositiveTests, Positive) | ||
| { | ||
| const auto& param = GetParam(); | ||
|
|
||
| EventEmitter<Poseidon2HashEvent> hash_event_emitter; | ||
| NoopEventEmitter<Poseidon2PermutationEvent> perm_event_emitter; | ||
| Poseidon2 poseidon2(hash_event_emitter, perm_event_emitter); | ||
|
|
||
| EventEmitter<MerkleCheckEvent> merkle_event_emitter; | ||
| MerkleCheck merkle_check(poseidon2, merkle_event_emitter); | ||
|
|
||
| NiceMock<MockRangeCheck> range_check; | ||
|
|
||
| EventEmitter<FieldGreaterThanEvent> field_gt_event_emitter; | ||
| FieldGreaterThan field_gt(range_check, field_gt_event_emitter); | ||
|
|
||
| EventEmitter<PublicDataTreeReadEvent> public_data_tree_read_event_emitter; | ||
| PublicDataTreeCheck public_data_tree_check_simulator( | ||
| poseidon2, merkle_check, field_gt, public_data_tree_read_event_emitter); | ||
|
|
||
| TestTraceContainer trace({ { { C::precomputed_first_row, 1 } } }); | ||
| Poseidon2TraceBuilder poseidon2_builder; | ||
| MerkleCheckTraceBuilder merkle_check_builder; | ||
| FieldGreaterThanTraceBuilder field_gt_builder; | ||
| PublicDataTreeReadTraceBuilder public_data_tree_read_builder; | ||
|
|
||
| FF low_leaf_hash = poseidon2.hash(param.low_leaf.get_hash_inputs()); | ||
| uint64_t leaf_index = 30; | ||
| std::vector<FF> sibling_path; | ||
| sibling_path.reserve(PUBLIC_DATA_TREE_HEIGHT); | ||
| for (size_t i = 0; i < PUBLIC_DATA_TREE_HEIGHT; ++i) { | ||
| sibling_path.emplace_back(i); | ||
| } | ||
| FF root = root_from_path(low_leaf_hash, leaf_index, sibling_path); | ||
|
|
||
| public_data_tree_check_simulator.assert_read( | ||
| param.leaf_slot, param.value, param.low_leaf, leaf_index, sibling_path, root); | ||
|
|
||
| poseidon2_builder.process_hash(hash_event_emitter.dump_events(), trace); | ||
| merkle_check_builder.process(merkle_event_emitter.dump_events(), trace); | ||
| field_gt_builder.process(field_gt_event_emitter.dump_events(), trace); | ||
| public_data_tree_read_builder.process(public_data_tree_read_event_emitter.dump_events(), trace); | ||
|
|
||
| check_relation<public_data_read>(trace); | ||
| } | ||
|
|
||
| INSTANTIATE_TEST_SUITE_P(PublicDataTreeReadConstrainingTest, | ||
| PublicDataReadPositiveTests, | ||
| ::testing::ValuesIn(positive_tests)); | ||
|
|
||
| TEST(PublicDataTreeReadConstrainingTest, NegativeExistsFlagCheck) | ||
| { | ||
| // Test constraint: sel * (SLOT_LOW_LEAF_SLOT_DIFF * (LEAF_EXISTS * (1 - slot_low_leaf_slot_diff_inv) + | ||
| // slot_low_leaf_slot_diff_inv) - 1 + LEAF_EXISTS) = 0 | ||
| TestTraceContainer trace({ | ||
| { { C::public_data_read_sel, 1 }, | ||
| { C::public_data_read_slot, 27 }, | ||
| { C::public_data_read_low_leaf_slot, 27 }, | ||
| { C::public_data_read_slot_low_leaf_slot_diff_inv, 0 }, | ||
| { C::public_data_read_leaf_not_exists, 0 } }, | ||
| { { C::public_data_read_sel, 1 }, | ||
| { C::public_data_read_slot, 28 }, | ||
| { C::public_data_read_low_leaf_slot, 27 }, | ||
| { C::public_data_read_slot_low_leaf_slot_diff_inv, FF(1).invert() }, | ||
| { C::public_data_read_leaf_not_exists, 1 } }, | ||
| }); | ||
|
|
||
| check_relation<public_data_read>(trace, public_data_read::SR_EXISTS_FLAG_CHECK); | ||
|
|
||
| trace.set(C::public_data_read_leaf_not_exists, 0, 1); | ||
|
|
||
| EXPECT_THROW_WITH_MESSAGE(check_relation<public_data_read>(trace, public_data_read::SR_EXISTS_FLAG_CHECK), | ||
| "EXISTS_FLAG_CHECK"); | ||
|
|
||
| trace.set(C::public_data_read_leaf_not_exists, 0, 0); | ||
| trace.set(C::public_data_read_leaf_not_exists, 1, 0); | ||
|
|
||
| EXPECT_THROW_WITH_MESSAGE(check_relation<public_data_read>(trace, public_data_read::SR_EXISTS_FLAG_CHECK), | ||
| "EXISTS_FLAG_CHECK"); | ||
| } | ||
|
|
||
| TEST(PublicDataTreeReadConstrainingTest, NegativeNextSlotIsZero) | ||
| { | ||
| // Test constraint: leaf_not_exists * (low_leaf_next_slot * (NEXT_SLOT_IS_ZERO * (1 - next_slot_inv) + | ||
| // next_slot_inv) - 1 + NEXT_SLOT_IS_ZERO) = 0 | ||
| TestTraceContainer trace({ | ||
| { | ||
| { C::public_data_read_leaf_not_exists, 1 }, | ||
| { C::public_data_read_low_leaf_next_slot, 0 }, | ||
| { C::public_data_read_next_slot_inv, 0 }, | ||
| { C::public_data_read_next_slot_is_nonzero, 0 }, | ||
| }, | ||
| { | ||
| { C::public_data_read_leaf_not_exists, 1 }, | ||
| { C::public_data_read_low_leaf_next_slot, 1 }, | ||
| { C::public_data_read_next_slot_inv, FF(1).invert() }, | ||
| { C::public_data_read_next_slot_is_nonzero, 1 }, | ||
| }, | ||
| }); | ||
|
|
||
| check_relation<public_data_read>(trace, public_data_read::SR_NEXT_SLOT_IS_ZERO_CHECK); | ||
|
|
||
| trace.set(C::public_data_read_next_slot_is_nonzero, 0, 1); | ||
|
|
||
| EXPECT_THROW_WITH_MESSAGE(check_relation<public_data_read>(trace, public_data_read::SR_NEXT_SLOT_IS_ZERO_CHECK), | ||
| "NEXT_SLOT_IS_ZERO_CHECK"); | ||
|
|
||
| trace.set(C::public_data_read_next_slot_is_nonzero, 0, 0); | ||
| trace.set(C::public_data_read_next_slot_is_nonzero, 1, 0); | ||
|
|
||
| EXPECT_THROW_WITH_MESSAGE(check_relation<public_data_read>(trace, public_data_read::SR_NEXT_SLOT_IS_ZERO_CHECK), | ||
| "NEXT_SLOT_IS_ZERO_CHECK"); | ||
| } | ||
|
|
||
| TEST(PublicDataTreeReadConstrainingTest, NegativeValueIsCorrect) | ||
| { | ||
| // Test constraint: leaf_not_exists * (low_leaf_next_slot * (NEXT_SLOT_IS_ZERO * (1 - next_slot_inv) + | ||
| // next_slot_inv) - 1 + NEXT_SLOT_IS_ZERO) = 0 | ||
| TestTraceContainer trace({ | ||
| { | ||
| { C::public_data_read_low_leaf_value, 27 }, | ||
| { C::public_data_read_leaf_not_exists, 0 }, | ||
| { C::public_data_read_value, 27 }, | ||
| }, | ||
| { | ||
| { C::public_data_read_low_leaf_value, 27 }, | ||
| { C::public_data_read_leaf_not_exists, 1 }, | ||
| { C::public_data_read_value, 0 }, | ||
| }, | ||
| }); | ||
|
|
||
| check_relation<public_data_read>(trace, public_data_read::SR_VALUE_IS_CORRECT); | ||
|
|
||
| // Invalid, if leaf exists, the value should be the low leaf value | ||
| trace.set(C::public_data_read_value, 0, 0); | ||
|
|
||
| EXPECT_THROW_WITH_MESSAGE(check_relation<public_data_read>(trace, public_data_read::SR_VALUE_IS_CORRECT), | ||
| "VALUE_IS_CORRECT"); | ||
|
|
||
| trace.set(C::public_data_read_value, 0, 27); | ||
| // Invalid, if leaf does not exists, the value should be zero | ||
| trace.set(C::public_data_read_value, 1, 27); | ||
|
|
||
| EXPECT_THROW_WITH_MESSAGE(check_relation<public_data_read>(trace, public_data_read::SR_VALUE_IS_CORRECT), | ||
| "VALUE_IS_CORRECT"); | ||
| } | ||
|
|
||
| } // namespace | ||
| } // namespace bb::avm2::constraining |
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need ./ ?