feat: zero leaf indexed tree (#285)

* feat: circuit now uses 0 hashes * feat: nullifier tree uses 0 as the empty hash * chore(bb): bump * chore(bb): update branch * fix: update 0 insertion * force build circuits * fix hashConstructor test + update snapshot * remove log * clean: remove force rebuild * fix: address nits * fix: include uncommitted comments * docs: uncommitted comments --------- Co-authored-by: cheethas <[email protected]> Co-authored-by: cheethas <[email protected]> Co-authored-by: spypsy <[email protected]>
AztecProtocol · Apr 19, 2023 · 1b8d40e · 1b8d40e
1 parent 6e2ec96
commit 1b8d40e
Show file tree

Hide file tree

Showing 10 changed files with 137 additions and 131 deletions.
diff --git a/circuits/cpp/barretenberg b/circuits/cpp/barretenberg
diff --git a/circuits/cpp/src/aztec3/circuits/rollup/base/native_base_rollup_circuit.cpp b/circuits/cpp/src/aztec3/circuits/rollup/base/native_base_rollup_circuit.cpp
@@ -23,16 +23,7 @@ namespace aztec3::circuits::rollup::native_base_rollup {
 
 const NT::fr EMPTY_COMMITMENTS_SUBTREE_ROOT = MerkleTree(PRIVATE_DATA_SUBTREE_DEPTH).root();
 const NT::fr EMPTY_CONTRACTS_SUBTREE_ROOT = MerkleTree(CONTRACT_SUBTREE_DEPTH).root();
-
-// Note: this is temporary until I work out how to encode a large fr in a constant
-NT::fr calculate_empty_nullifier_subtree_root()
-{
-    MerkleTree empty_nullifier_tree = MerkleTree(NULLIFIER_SUBTREE_DEPTH);
-    for (size_t i = 0; i < KERNEL_NEW_NULLIFIERS_LENGTH * 2; i++) {
-        empty_nullifier_tree.update_element(i, NullifierLeaf{ .value = 0, .nextIndex = 0, .nextValue = 0 }.hash());
-    }
-    return empty_nullifier_tree.root();
-}
+const NT::fr EMPTY_NULLIFIER_SUBTREE_ROOT = MerkleTree(NULLIFIER_SUBTREE_DEPTH).root();
 
 // TODO: can we aggregate proofs if we do not have a working circuit impl
 
@@ -242,9 +233,11 @@ NT::fr create_nullifier_subtree(std::array<NullifierLeaf, KERNEL_NEW_NULLIFIERS_
     MerkleTree nullifier_subtree = MerkleTree(NULLIFIER_SUBTREE_DEPTH);
     for (size_t i = 0; i < nullifier_leaves.size(); i++) {
         // check if the nullifier is zero, if so dont insert
-        // if (nullifier_leaves[i].value != fr(0)) { // TODO: reinsert after 0 is accounted for
-        nullifier_subtree.update_element(i, nullifier_leaves[i].hash());
-        // }
+        if (uint256_t(nullifier_leaves[i].value) == uint256_t(0)) {
+            nullifier_subtree.update_element(i, fr::zero());
+        } else {
+            nullifier_subtree.update_element(i, nullifier_leaves[i].hash());
+        }
     }
 
     return nullifier_subtree.root();
@@ -406,10 +399,6 @@ AppendOnlySnapshot check_nullifier_tree_non_membership_and_insert_to_tree(DummyC
 
 BaseOrMergeRollupPublicInputs base_rollup_circuit(DummyComposer& composer, BaseRollupInputs const& baseRollupInputs)
 {
-    // TODO: move this into a constant - calc empty nullifier_subtree hash
-    // calc empty subtree root
-    const NT::fr EMPTY_NULLIFIER_SUBTREE_ROOT = calculate_empty_nullifier_subtree_root();
-
     // Verify the previous kernel proofs
     for (size_t i = 0; i < 2; i++) {
         NT::Proof proof = baseRollupInputs.kernel_data[i].proof;

diff --git a/circuits/cpp/src/aztec3/circuits/rollup/base/nullifier_tree_testing_harness.cpp b/circuits/cpp/src/aztec3/circuits/rollup/base/nullifier_tree_testing_harness.cpp
@@ -26,51 +26,6 @@ bool check_has_less_than(std::vector<fr> const& values, fr const& value)
     return false;
 }
 
-// TODO: test
-fr NullifierMemoryTreeTestingHarness::append_value(fr const& value)
-{
-    // If the value is 0, then we force insert the value to increase the size of the tree
-    // TODO: this is a hack
-    if (value == 0) {
-        nullifier_leaf new_leaf = { .value = 0, .nextIndex = 0, .nextValue = 0 };
-        auto new_leaf_hash = new_leaf.hash();
-        leaves_.push_back(new_leaf);
-        size_t new_leaf_index = leaves_.size() - 1;
-        auto root = update_element(new_leaf_index, new_leaf_hash);
-        return root;
-    }
-
-    // Find the leaf with the value closest and less than `value`
-    size_t current;
-    bool is_already_present;
-    std::tie(current, is_already_present) = find_closest_leaf(leaves_, value);
-
-    nullifier_leaf new_leaf = { .value = value,
-                                .nextIndex = leaves_[current].nextIndex,
-                                .nextValue = leaves_[current].nextValue };
-    if (!is_already_present) {
-        // Update the current leaf to point it to the new leaf
-        leaves_[current].nextIndex = leaves_.size();
-        leaves_[current].nextValue = value;
-
-        // Insert the new leaf with (nextIndex, nextValue) of the current leaf
-        leaves_.push_back(new_leaf);
-    }
-
-    // Update the old leaf in the tree
-    auto old_leaf_hash = leaves_[current].hash();
-    size_t old_leaf_index = current;
-    auto root = update_element(old_leaf_index, old_leaf_hash);
-
-    // Insert the new leaf in the tree
-    auto new_leaf_hash = new_leaf.hash();
-
-    size_t new_leaf_index = is_already_present ? old_leaf_index : leaves_.size() - 1;
-    root = update_element(new_leaf_index, new_leaf_hash);
-
-    return root;
-}
-
 // handle synthetic membership assertions
 std::tuple<std::vector<nullifier_leaf>, std::vector<std::vector<fr>>, std::vector<uint32_t>>
 NullifierMemoryTreeTestingHarness::circuit_prep_batch_insert(std::vector<fr> const& values)
@@ -124,7 +79,6 @@ NullifierMemoryTreeTestingHarness::circuit_prep_batch_insert(std::vector<fr> con
         }
         // If there is a lower value in the tree, we need to check the current low nullifiers for one that can be used
         if (has_less_than) {
-
             for (size_t j = 0; j < pending_insertion_tree.size(); ++j) {
                 // Skip checking empty values
                 if (pending_insertion_tree[j].value == 0) {
@@ -147,7 +101,7 @@ NullifierMemoryTreeTestingHarness::circuit_prep_batch_insert(std::vector<fr> con
                 }
             }
 
-            // empty low nullifier
+            // add empty low nullifier
             sibling_paths.push_back(empty_sp);
             low_nullifier_indexes.push_back(empty_index);
             low_nullifiers.push_back(empty_leaf);
@@ -160,7 +114,7 @@ NullifierMemoryTreeTestingHarness::circuit_prep_batch_insert(std::vector<fr> con
                 prev_nodes->second.push_back(new_value);
             }
 
-            nullifier_leaf low_nullifier = leaves_[current];
+            nullifier_leaf low_nullifier = leaves_[current].unwrap();
             std::vector<fr> sibling_path = this->get_sibling_path(current);
 
             sibling_paths.push_back(sibling_path);
@@ -185,7 +139,7 @@ NullifierMemoryTreeTestingHarness::circuit_prep_batch_insert(std::vector<fr> con
 void NullifierMemoryTreeTestingHarness::update_element_in_place(size_t index, nullifier_leaf leaf)
 {
     // Find the leaf with the value closest and less than `value`
-    this->leaves_[index] = leaf;
+    this->leaves_[index].set(leaf);
     update_element(index, leaf.hash());
 }
 
@@ -197,8 +151,8 @@ std::pair<nullifier_leaf, size_t> NullifierMemoryTreeTestingHarness::find_lower(
 
     // TODO: handle is already present case
     if (!is_already_present) {
-        return std::make_pair(leaves_[current], current);
+        return std::make_pair(leaves_[current].unwrap(), current);
     } else {
-        return std::make_pair(leaves_[current], current);
+        return std::make_pair(leaves_[current].unwrap(), current);
     }
 }
diff --git a/circuits/cpp/src/aztec3/circuits/rollup/base/nullifier_tree_testing_harness.hpp b/circuits/cpp/src/aztec3/circuits/rollup/base/nullifier_tree_testing_harness.hpp
@@ -28,9 +28,6 @@ class NullifierMemoryTreeTestingHarness : public proof_system::plonk::stdlib::me
     // Get the value immediately lower than the given value
     std::pair<nullifier_leaf, size_t> find_lower(fr const& value);
 
-    // Append a value to the tree, even zeros
-    fr append_value(fr const& value);
-
     // Utilities to inspect tree
     fr total_size() const { return total_size_; }
     fr depth() const { return depth_; }

diff --git a/circuits/cpp/src/aztec3/circuits/rollup/base/utils.cpp b/circuits/cpp/src/aztec3/circuits/rollup/base/utils.cpp
@@ -68,7 +68,7 @@ NullifierMemoryTreeTestingHarness get_initial_nullifier_tree(std::vector<fr> ini
 {
     NullifierMemoryTreeTestingHarness nullifier_tree = NullifierMemoryTreeTestingHarness(NULLIFIER_TREE_HEIGHT);
     for (size_t i = 0; i < initial_values.size(); ++i) {
-        nullifier_tree.append_value(initial_values[i]);
+        nullifier_tree.update_element(initial_values[i]);
     }
     return nullifier_tree;
 }
@@ -158,7 +158,7 @@ generate_nullifier_tree_testing_values(BaseRollupInputs<NT> rollupInputs,
             new_nullifiers_kernel_2[i - KERNEL_NEW_NULLIFIERS_LENGTH] = insertion_val;
         }
         insertion_values.push_back(insertion_val);
-        reference_tree.append_value(insertion_val);
+        reference_tree.update_element(insertion_val);
     }
 
     // Get the hash paths etc from the insertion values

diff --git a/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap b/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap
@@ -102,38 +102,38 @@ Fr {
 exports[`abis wasm bindings hash constructor info 1`] = `
 {
   "data": [
-    33,
-    79,
-    48,
-    128,
-    228,
+    36,
+    203,
+    121,
+    198,
+    238,
+    246,
+    39,
+    11,
+    209,
+    251,
+    234,
+    135,
+    70,
     0,
+    68,
+    226,
+    184,
+    31,
     77,
-    227,
-    224,
-    187,
-    234,
-    166,
+    126,
+    81,
+    123,
+    23,
+    198,
     162,
-    215,
-    250,
-    187,
-    135,
-    156,
-    67,
-    135,
-    234,
-    67,
-    223,
-    239,
-    172,
-    228,
-    95,
-    105,
-    107,
-    101,
-    207,
-    241,
+    185,
+    199,
+    254,
+    183,
+    90,
+    167,
+    9,
   ],
   "type": "Buffer",
 }

diff --git a/yarn-project/circuits.js/src/abis/abis.test.ts b/yarn-project/circuits.js/src/abis/abis.test.ts
@@ -68,7 +68,8 @@ describe('abis wasm bindings', () => {
 
   it('hash constructor info', async () => {
     const functionData = new FunctionData(Buffer.alloc(4), true, true);
-    const args = [new Fr(0n), new Fr(1n)];
+    // args needs to have a FIXED length of 8, due to a circuit constant `aztec3::ARGS_SIZE`.
+    const args = [new Fr(0n), new Fr(1n), new Fr(0n), new Fr(1n), new Fr(0n), new Fr(1n), new Fr(0n), new Fr(1n)];
     const vkHash = Buffer.alloc(32);
     const res = await hashConstructor(wasm, functionData, args, vkHash);
     expect(res).toMatchSnapshot();
+67 −2		cpp/src/barretenberg/stdlib/merkle_tree/nullifier_tree/nullifier_leaf.hpp
+23 −11		cpp/src/barretenberg/stdlib/merkle_tree/nullifier_tree/nullifier_memory_tree.cpp
+7 −3		cpp/src/barretenberg/stdlib/merkle_tree/nullifier_tree/nullifier_memory_tree.hpp
+192 −24		cpp/src/barretenberg/stdlib/merkle_tree/nullifier_tree/nullifier_memory_tree.test.cpp
+14 −9		cpp/src/barretenberg/stdlib/merkle_tree/nullifier_tree/nullifier_tree.cpp
+1 −1		cpp/src/barretenberg/stdlib/merkle_tree/nullifier_tree/nullifier_tree.hpp
-Original file line number
+Diff line change
@@ Expand Up / @@ -102,38 +102,38 @@ Fr { @@
     exports[`abis wasm bindings hash constructor info 1`] = `
     {
       "data": [
-,
-,
-,
-,
-,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
 ,
+,
+,
+,
+,
 ,
-,
-,
-,
-,
-,
+,
+,
+,
+,
+,
 ,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
+,
+,
+,
+,
+,
+,
+,
       ],
       "type": "Buffer",
     }
@@ Expand Down @@