From 87cb6ee43031630a93c6408821d15574ade0157e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Fri, 29 Nov 2024 17:45:39 +0100 Subject: [PATCH 1/6] feat: Avoid inserting an empty leaf in indexed trees on update (#10281) Previously, we were always appending an empty leaf when we encountered an update in the public data tree. After this PR public data tree insertions don't add an empty leaf when the write is an update. --- .../content_addressed_indexed_tree.hpp | 120 +++++++++++------- .../merkle_tree/indexed_tree/indexed_leaf.hpp | 2 +- .../vm/avm/trace/gadgets/merkle_tree.cpp | 8 +- .../src/base/components/public_data_tree.nr | 15 +-- .../types/src/data/public_data_tree_leaf.nr | 8 +- .../types/src/merkle_tree/indexed_tree.nr | 52 ++++---- .../indexed_tree/check_valid_low_leaf.nr | 7 + .../types/src/merkle_tree/leaf_preimage.nr | 8 +- .../types/src/merkle_tree/membership.nr | 7 + .../simulator/src/avm/avm_simulator.test.ts | 6 +- .../simulator/src/avm/avm_tree.test.ts | 87 +++++++------ yarn-project/simulator/src/avm/avm_tree.ts | 113 +++++++++-------- .../simulator/src/avm/journal/journal.ts | 25 ++-- 13 files changed, 267 insertions(+), 191 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp index 197f784d59f..e1a47ec5351 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp @@ -252,18 +252,20 @@ class ContentAddressedIndexedTree : public ContentAddressedAppendOnlyTree> new_leaf; }; struct SequentialInsertionGenerationResponse { - std::shared_ptr> updates_to_perform; + std::vector updates_to_perform; index_t highest_index; }; using SequentialInsertionGenerationCallback = - std::function&)>; + std::function&)>; void generate_sequential_insertions(const std::vector& values, const SequentialInsertionGenerationCallback& completion); @@ -1422,6 +1424,14 @@ void ContentAddressedIndexedTree::add_or_update_values_seq const AddSequentiallyCompletionCallbackWithWitness& completion, bool capture_witness) { + + // This struct is used to collect some state from the asynchronous operations we are about to perform + struct IntermediateResults { + std::vector updates_to_perform; + size_t appended_leaves = 0; + }; + auto results = std::make_shared(); + auto on_error = [=](const std::string& message) { try { TypedResponse> response; @@ -1443,7 +1453,7 @@ void ContentAddressedIndexedTree::add_or_update_values_seq ReadTransactionPtr tx = store_->create_read_transaction(); store_->get_meta(meta, *tx, true); - index_t new_total_size = values.size() + meta.size; + index_t new_total_size = results->appended_leaves + meta.size; meta.size = new_total_size; meta.root = store_->get_current_root(*tx, true); @@ -1452,23 +1462,29 @@ void ContentAddressedIndexedTree::add_or_update_values_seq if (capture_witness) { // Split results->update_witnesses between low_leaf_witness_data and insertion_witness_data - // Currently we always insert an empty leaf, even if it's an update, so we can split based - // on the index of the witness data response.inner.insertion_witness_data = std::make_shared>>(); - ; + response.inner.insertion_witness_data->reserve(results->updates_to_perform.size()); + response.inner.low_leaf_witness_data = std::make_shared>>(); - ; - - for (size_t i = 0; i < updates_completion_response.inner.update_witnesses->size(); ++i) { - LeafUpdateWitnessData& witness_data = - updates_completion_response.inner.update_witnesses->at(i); - // If even, it's a low leaf, if odd, it's an insertion witness - if (i % 2 == 0) { - response.inner.low_leaf_witness_data->push_back(witness_data); + response.inner.low_leaf_witness_data->reserve(results->updates_to_perform.size()); + + size_t current_witness_index = 0; + for (size_t i = 0; i < results->updates_to_perform.size(); ++i) { + LeafUpdateWitnessData low_leaf_witness = + updates_completion_response.inner.update_witnesses->at(current_witness_index++); + response.inner.low_leaf_witness_data->push_back(low_leaf_witness); + + // If this update has an insertion, append the real witness + if (results->updates_to_perform.at(i).new_leaf.has_value()) { + LeafUpdateWitnessData insertion_witness = + updates_completion_response.inner.update_witnesses->at(current_witness_index++); + response.inner.insertion_witness_data->push_back(insertion_witness); } else { - response.inner.insertion_witness_data->push_back(witness_data); + // If it's an update, append an empty witness + response.inner.insertion_witness_data->push_back(LeafUpdateWitnessData{ + .leaf = IndexedLeafValueType::empty(), .index = 0, .path = std::vector(depth_) }); } } } @@ -1480,23 +1496,33 @@ void ContentAddressedIndexedTree::add_or_update_values_seq // This signals the completion of the insertion data generation // Here we'll perform all updates to the tree SequentialInsertionGenerationCallback insertion_generation_completed = - [=, this](const TypedResponse& insertion_response) { + [=, this](TypedResponse& insertion_response) { if (!insertion_response.success) { on_error(insertion_response.message); return; } std::shared_ptr> flat_updates = std::make_shared>(); - for (size_t i = 0; i < insertion_response.inner.updates_to_perform->size(); ++i) { - InsertionUpdates& insertion_update = insertion_response.inner.updates_to_perform->at(i); + flat_updates->reserve(insertion_response.inner.updates_to_perform.size() * 2); + + for (size_t i = 0; i < insertion_response.inner.updates_to_perform.size(); ++i) { + InsertionUpdates& insertion_update = insertion_response.inner.updates_to_perform.at(i); flat_updates->push_back(insertion_update.low_leaf_update); - flat_updates->push_back(LeafUpdate{ - .leaf_index = insertion_update.new_leaf_index, - .updated_leaf = insertion_update.new_leaf, - .original_leaf = IndexedLeafValueType::empty(), - }); + if (insertion_update.new_leaf.has_value()) { + results->appended_leaves++; + IndexedLeafValueType new_leaf; + index_t new_leaf_index = 0; + std::tie(new_leaf, new_leaf_index) = insertion_update.new_leaf.value(); + flat_updates->push_back(LeafUpdate{ + .leaf_index = new_leaf_index, + .updated_leaf = new_leaf, + .original_leaf = IndexedLeafValueType::empty(), + }); + } } - + // We won't use anymore updates_to_perform + results->updates_to_perform = std::move(insertion_response.inner.updates_to_perform); + assert(insertion_response.inner.updates_to_perform.size() == 0); if (capture_witness) { perform_updates(flat_updates->size(), flat_updates, final_completion); return; @@ -1514,27 +1540,12 @@ void ContentAddressedIndexedTree::generate_sequential_inse { execute_and_report( [=, this](TypedResponse& response) { - response.inner.highest_index = 0; - response.inner.updates_to_perform = std::make_shared>(); - TreeMeta meta; ReadTransactionPtr tx = store_->create_read_transaction(); store_->get_meta(meta, *tx, true); RequestContext requestContext; requestContext.includeUncommitted = true; - // Ensure that the tree is not going to be overfilled - index_t new_total_size = values.size() + meta.size; - if (new_total_size > max_size_) { - throw std::runtime_error(format("Unable to insert values into tree ", - meta.name, - " new size: ", - new_total_size, - " max size: ", - max_size_)); - } - // The highest index touched will be the last leaf index, since we append a zero for updates - response.inner.highest_index = new_total_size - 1; requestContext.root = store_->get_current_root(*tx, true); // Fetch the frontier (non empty nodes to the right) of the tree. This will ensure that perform_updates or // perform_updates_without_witness has all the cached nodes it needs to perform the insertions. See comment @@ -1543,12 +1554,15 @@ void ContentAddressedIndexedTree::generate_sequential_inse find_leaf_hash(meta.size - 1, requestContext, *tx, true); } + index_t current_size = meta.size; + for (size_t i = 0; i < values.size(); ++i) { const LeafValueType& new_payload = values[i]; + // TODO(Alvaro) - Rethink this. I think it's fine for us to interpret empty values as a regular update + // (it'd empty out the payload of the zero leaf) if (new_payload.is_empty()) { continue; } - index_t index_of_new_leaf = i + meta.size; fr value = new_payload.get_key(); // This gives us the leaf that need updating @@ -1595,17 +1609,17 @@ void ContentAddressedIndexedTree::generate_sequential_inse .updated_leaf = IndexedLeafValueType::empty(), .original_leaf = low_leaf, }, - .new_leaf = IndexedLeafValueType::empty(), - .new_leaf_index = index_of_new_leaf, + .new_leaf = std::nullopt, }; if (!is_already_present) { // Update the current leaf to point it to the new leaf IndexedLeafValueType new_leaf = IndexedLeafValueType(new_payload, low_leaf.nextIndex, low_leaf.nextValue); - + index_t index_of_new_leaf = current_size; low_leaf.nextIndex = index_of_new_leaf; low_leaf.nextValue = value; + current_size++; // Cache the new leaf store_->set_leaf_key_at_index(index_of_new_leaf, new_leaf); store_->put_cached_leaf_by_index(index_of_new_leaf, new_leaf); @@ -1613,7 +1627,7 @@ void ContentAddressedIndexedTree::generate_sequential_inse store_->put_cached_leaf_by_index(low_leaf_index, low_leaf); insertion_update.low_leaf_update.updated_leaf = low_leaf; - insertion_update.new_leaf = new_leaf; + insertion_update.new_leaf = std::pair(new_leaf, index_of_new_leaf); } else if (IndexedLeafValueType::is_updateable()) { // Update the current leaf's value, don't change it's link IndexedLeafValueType replacement_leaf = @@ -1631,8 +1645,20 @@ void ContentAddressedIndexedTree::generate_sequential_inse " is already present")); } - response.inner.updates_to_perform->push_back(insertion_update); + response.inner.updates_to_perform.push_back(insertion_update); + } + + // Ensure that the tree is not going to be overfilled + if (current_size > max_size_) { + throw std::runtime_error(format("Unable to insert values into tree ", + meta.name, + " new size: ", + current_size, + " max size: ", + max_size_)); } + // The highest index touched will be current_size - 1 + response.inner.highest_index = current_size - 1; }, completion); } diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/indexed_leaf.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/indexed_leaf.hpp index 615f2ce4cf2..bc13a93e598 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/indexed_leaf.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/indexed_leaf.hpp @@ -107,7 +107,7 @@ struct PublicDataLeafValue { fr get_key() const { return slot; } - bool is_empty() const { return slot == fr::zero(); } + bool is_empty() const { return slot == fr::zero() && value == fr::zero(); } std::vector get_hash_inputs(fr nextValue, fr nextIndex) const { diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp index 00171489efa..32a1e7eec2b 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp @@ -102,12 +102,8 @@ FF AvmMerkleTreeTraceBuilder::perform_storage_write([[maybe_unused]] uint32_t cl // We update the low value low_preimage.value = value; FF low_preimage_hash = unconstrained_hash_public_data_preimage(low_preimage); - // Update the low leaf - this will be returned in future - [[maybe_unused]] FF root = - unconstrained_update_leaf_index(low_preimage_hash, static_cast(low_index), low_path); - // TEMPORARY UNTIL WE CHANGE HOW UPDATES WORK - // Insert a zero leaf at the insertion index - return unconstrained_update_leaf_index(FF::zero(), static_cast(insertion_index), insertion_path); + // Update the low leaf + return unconstrained_update_leaf_index(low_preimage_hash, static_cast(low_index), low_path); } // The new leaf for an insertion is PublicDataTreeLeafPreimage new_preimage{ diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_data_tree.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_data_tree.nr index e3d152ba98d..4afaf9c75ce 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_data_tree.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_data_tree.nr @@ -57,16 +57,11 @@ pub(crate) fn public_data_tree_insert( }, |write: PublicDataTreeLeaf, low_preimage: PublicDataTreeLeafPreimage| { // Build insertion leaf - let is_update = low_preimage.slot == write.slot; - if is_update { - PublicDataTreeLeafPreimage::empty() - } else { - PublicDataTreeLeafPreimage { - slot: write.slot, - value: write.value, - next_slot: low_preimage.next_slot, - next_index: low_preimage.next_index, - } + PublicDataTreeLeafPreimage { + slot: write.slot, + value: write.value, + next_slot: low_preimage.next_slot, + next_index: low_preimage.next_index, } }, ) diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/data/public_data_tree_leaf.nr b/noir-projects/noir-protocol-circuits/crates/types/src/data/public_data_tree_leaf.nr index e97a2161416..9cae7a6a675 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/data/public_data_tree_leaf.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/data/public_data_tree_leaf.nr @@ -1,4 +1,4 @@ -use crate::traits::Empty; +use crate::{merkle_tree::leaf_preimage::IndexedTreeLeafValue, traits::Empty}; pub struct PublicDataTreeLeaf { pub slot: Field, @@ -17,6 +17,12 @@ impl Empty for PublicDataTreeLeaf { } } +impl IndexedTreeLeafValue for PublicDataTreeLeaf { + fn get_key(self) -> Field { + self.slot + } +} + impl PublicDataTreeLeaf { pub fn is_empty(self) -> bool { (self.slot == 0) & (self.value == 0) diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree.nr b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree.nr index d6d2a363fd3..6b1be23c529 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree.nr @@ -3,6 +3,7 @@ pub mod check_valid_low_leaf; use crate::{ abis::append_only_tree_snapshot::AppendOnlyTreeSnapshot, merkle_tree::{ + leaf_preimage::{IndexedTreeLeafPreimage, IndexedTreeLeafValue}, membership::{assert_check_membership, MembershipWitness}, root::{calculate_empty_tree_root, calculate_subtree_root, root_from_sibling_path}, }, @@ -112,14 +113,14 @@ pub fn insert( build_insertion_leaf: fn(Value, Leaf) -> Leaf, ) -> AppendOnlyTreeSnapshot where - Value: Eq + Empty, - Leaf: Hash + Empty, + Value: IndexedTreeLeafValue, + Leaf: IndexedTreeLeafPreimage, { assert(is_valid_low_leaf(low_leaf_preimage, value), "Invalid low leaf"); // perform membership check for the low leaf against the original root assert_check_membership( - low_leaf_preimage.hash(), + low_leaf_preimage.as_leaf(), low_leaf_membership_witness.leaf_index, low_leaf_membership_witness.sibling_path, snapshot.root, @@ -129,29 +130,34 @@ where let updated_low_leaf = update_low_leaf(low_leaf_preimage, value, snapshot.next_available_leaf_index); + // Update low leaf snapshot.root = root_from_sibling_path( - updated_low_leaf.hash(), + updated_low_leaf.as_leaf(), low_leaf_membership_witness.leaf_index, low_leaf_membership_witness.sibling_path, ); - let insertion_leaf = build_insertion_leaf(value, low_leaf_preimage); - - assert_check_membership( - 0, - snapshot.next_available_leaf_index as Field, - insertion_sibling_path, - snapshot.root, - ); - - // Calculate the new root - snapshot.root = root_from_sibling_path( - insertion_leaf.hash(), - snapshot.next_available_leaf_index as Field, - insertion_sibling_path, - ); - - snapshot.next_available_leaf_index += 1; - - snapshot + if low_leaf_preimage.get_key() == value.get_key() { + // If it's an update, we don't need to insert the new leaf and advance the tree + snapshot + } else { + let insertion_leaf = build_insertion_leaf(value, low_leaf_preimage); + assert_check_membership( + 0, + snapshot.next_available_leaf_index as Field, + insertion_sibling_path, + snapshot.root, + ); + + // Calculate the new root + snapshot.root = root_from_sibling_path( + insertion_leaf.as_leaf(), + snapshot.next_available_leaf_index as Field, + insertion_sibling_path, + ); + + snapshot.next_available_leaf_index += 1; + + snapshot + } } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree/check_valid_low_leaf.nr b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree/check_valid_low_leaf.nr index 6c454c5f583..9a42a21dafe 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree/check_valid_low_leaf.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree/check_valid_low_leaf.nr @@ -16,12 +16,19 @@ mod tests { indexed_tree::check_valid_low_leaf::assert_check_valid_low_leaf, leaf_preimage::IndexedTreeLeafPreimage, }; + use crate::traits::Empty; struct TestLeafPreimage { value: Field, next_value: Field, } + impl Empty for TestLeafPreimage { + fn empty() -> Self { + Self { value: 0, next_value: 0 } + } + } + impl IndexedTreeLeafPreimage for TestLeafPreimage { fn get_key(self) -> Field { self.value diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/leaf_preimage.nr b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/leaf_preimage.nr index f33dd072d96..dab825f5664 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/leaf_preimage.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/leaf_preimage.nr @@ -1,10 +1,16 @@ +use crate::traits::Empty; + pub trait LeafPreimage { fn get_key(self) -> Field; fn as_leaf(self) -> Field; } -pub trait IndexedTreeLeafPreimage { +pub trait IndexedTreeLeafPreimage: Empty { fn get_key(self) -> Field; fn get_next_key(self) -> Field; fn as_leaf(self) -> Field; } + +pub trait IndexedTreeLeafValue: Eq + Empty { + fn get_key(self) -> Field; +} diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/membership.nr b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/membership.nr index 178449af97d..34ab55a678d 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/membership.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/membership.nr @@ -93,6 +93,7 @@ mod tests { }, tests::merkle_tree_utils::NonEmptyMerkleTree, }; + use crate::traits::Empty; use std::hash::pedersen_hash; struct TestLeafPreimage { @@ -100,6 +101,12 @@ mod tests { next_value: Field, } + impl Empty for TestLeafPreimage { + fn empty() -> Self { + TestLeafPreimage { value: 0, next_value: 0 } + } + } + impl LeafPreimage for TestLeafPreimage { fn get_key(self) -> Field { self.value diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 06811af1557..72889ea63c1 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -1178,7 +1178,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { const { preimage: lowLeafPreimage, index: lowLeafIndex, - update: leafAlreadyPresent, + alreadyPresent: leafAlreadyPresent, } = await ephemeralForest.getLeafOrLowLeafInfo( MerkleTreeId.PUBLIC_DATA_TREE, leafSlot0, @@ -1223,7 +1223,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { const { preimage: lowLeafPreimage, index: lowLeafIndex, - update: leafAlreadyPresent, + alreadyPresent: leafAlreadyPresent, } = await ephemeralForest.getLeafOrLowLeafInfo( MerkleTreeId.PUBLIC_DATA_TREE, leafSlot0, @@ -1294,7 +1294,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { const { preimage: lowLeafPreimage, index: lowLeafIndex, - update: leafAlreadyPresent, + alreadyPresent: leafAlreadyPresent, } = await ephemeralForest.getLeafOrLowLeafInfo( MerkleTreeId.PUBLIC_DATA_TREE, leafSlot0, diff --git a/yarn-project/simulator/src/avm/avm_tree.test.ts b/yarn-project/simulator/src/avm/avm_tree.test.ts index aa0ab6bc07b..47f88232828 100644 --- a/yarn-project/simulator/src/avm/avm_tree.test.ts +++ b/yarn-project/simulator/src/avm/avm_tree.test.ts @@ -1,15 +1,15 @@ import { - type BatchInsertionResult, type IndexedTreeId, MerkleTreeId, type MerkleTreeWriteOperations, + type SequentialInsertionResult, } from '@aztec/circuit-types'; import { NOTE_HASH_TREE_HEIGHT, NULLIFIER_SUBTREE_HEIGHT, - type NULLIFIER_TREE_HEIGHT, + NULLIFIER_TREE_HEIGHT, type NullifierLeafPreimage, - type PUBLIC_DATA_TREE_HEIGHT, + PUBLIC_DATA_TREE_HEIGHT, PublicDataTreeLeaf, type PublicDataTreeLeafPreimage, } from '@aztec/circuits.js'; @@ -18,11 +18,16 @@ import { Fr } from '@aztec/foundation/fields'; import { type IndexedTreeLeafPreimage } from '@aztec/foundation/trees'; import { openTmpStore } from '@aztec/kv-store/utils'; import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; -import { MerkleTrees } from '@aztec/world-state'; +import { MerkleTrees, NativeWorldStateService } from '@aztec/world-state'; -import { AvmEphemeralForest, EphemeralAvmTree, type IndexedInsertionResult } from './avm_tree.js'; +import { + AvmEphemeralForest, + EphemeralAvmTree, + type IndexedInsertResult, + type IndexedUpsertResult, +} from './avm_tree.js'; -let worldStateTrees: MerkleTrees; +let mainState: MerkleTreeWriteOperations; let copyState: MerkleTreeWriteOperations; // Up to 64 dummy note hashes let noteHashes: Fr[]; @@ -37,8 +42,9 @@ let getSiblingIndex = 21n; // Helper to check the equality of the insertion results (low witness, insertion path) const checkEqualityOfInsertionResults = ( - containerResults: IndexedInsertionResult[], - wsResults: BatchInsertionResult[], + containerResults: IndexedUpsertResult[] | IndexedInsertResult[], + wsResults: SequentialInsertionResult[], + treeHeight: number, ) => { if (containerResults.length !== wsResults.length) { throw new Error('Results length mismatch'); @@ -49,40 +55,41 @@ const checkEqualityOfInsertionResults = ( expect(containerResult.lowWitness.siblingPath).toEqual(wsResult.lowLeavesWitnessData![0].siblingPath.toFields()); expect(containerResult.lowWitness.index).toEqual(wsResult.lowLeavesWitnessData![0].index); expect(containerResult.lowWitness.preimage).toEqual(wsResult.lowLeavesWitnessData![0].leafPreimage); - expect(containerResult.insertionPath).toEqual(wsResult.newSubtreeSiblingPath.toFields()); + if ('update' in containerResult && containerResult.update) { + expect(Array(treeHeight).fill(Fr.ZERO)).toEqual(wsResult.insertionWitnessData[0].siblingPath.toFields()); + } else { + expect(containerResult.insertionPath).toEqual(wsResult.insertionWitnessData[0].siblingPath.toFields()); + } } }; const getWorldStateRoot = async (treeId: MerkleTreeId) => { - return (await worldStateTrees.getTreeInfo(treeId, /*includeUncommitted=*/ true)).root; + return (await mainState.getTreeInfo(treeId)).root; }; const getWorldStateSiblingPath = (treeId: MerkleTreeId, index: bigint) => { - return worldStateTrees.getSiblingPath(treeId, index, /*includeUncommitted=*/ true); + return mainState.getSiblingPath(treeId, index); }; const publicDataInsertWorldState = ( slot: Fr, value: Fr, -): Promise> => { - return worldStateTrees.batchInsert( - MerkleTreeId.PUBLIC_DATA_TREE, - [new PublicDataTreeLeaf(slot, value).toBuffer()], - 0, - ); +): Promise> => { + return mainState.sequentialInsert(MerkleTreeId.PUBLIC_DATA_TREE, [new PublicDataTreeLeaf(slot, value).toBuffer()]); }; const nullifierInsertWorldState = ( nullifier: Fr, -): Promise> => { - return worldStateTrees.batchInsert(MerkleTreeId.NULLIFIER_TREE, [nullifier.toBuffer()], 0); +): Promise> => { + return mainState.sequentialInsert(MerkleTreeId.NULLIFIER_TREE, [nullifier.toBuffer()]); }; // Set up some recurring state for the tests beforeEach(async () => { - const store = openTmpStore(true); - worldStateTrees = await MerkleTrees.new(store, new NoopTelemetryClient()); - copyState = await worldStateTrees.fork(); + const worldState = await NativeWorldStateService.tmp(); + + mainState = await worldState.fork(); + copyState = await worldState.fork(); noteHashes = Array.from({ length: 64 }, (_, i) => new Fr(i)); // We do + 128 since the first 128 leaves are already filled in the indexed trees (nullifier, public data) @@ -125,7 +132,7 @@ describe('Simple Note Hash Consistency', () => { for (const noteHash of noteHashes) { treeContainer.appendNoteHash(noteHash); } - await worldStateTrees.appendLeaves(treeId, noteHashes); + await mainState.appendLeaves(treeId, noteHashes); // Check that the roots are consistent const wsRoot = await getWorldStateRoot(treeId); @@ -152,7 +159,7 @@ describe('Simple Note Hash Consistency', () => { } // Build a worldstateDB with all the note hashes - await worldStateTrees.appendLeaves(treeId, preInserted.concat(postInserted)); + await mainState.appendLeaves(treeId, preInserted.concat(postInserted)); // Check that the roots are consistent const wsRoot = await getWorldStateRoot(treeId); @@ -174,8 +181,8 @@ describe('Simple Note Hash Consistency', () => { describe('Simple Public Data Consistency', () => { const treeId = MerkleTreeId.PUBLIC_DATA_TREE as IndexedTreeId; - let containerInsertionResults: IndexedInsertionResult[] = []; - let worldStateInsertionResults: BatchInsertionResult[] = []; + let containerInsertionResults: IndexedUpsertResult[] = []; + let worldStateInsertionResults: SequentialInsertionResult[] = []; // We need to zero out between tests afterEach(() => { @@ -199,7 +206,7 @@ describe('Simple Public Data Consistency', () => { expect(computedRoot.toBuffer()).toEqual(wsRoot); // Check that all the accumulated insertion results match - checkEqualityOfInsertionResults(containerInsertionResults, worldStateInsertionResults); + checkEqualityOfInsertionResults(containerInsertionResults, worldStateInsertionResults, PUBLIC_DATA_TREE_HEIGHT); }); it('Should fork a prefilled tree and check consistency', async () => { @@ -267,14 +274,14 @@ describe('Simple Public Data Consistency', () => { expect(computedRoot.toBuffer()).toEqual(wsRoot); // Check the insertion results match - checkEqualityOfInsertionResults(containerInsertionResults, worldStateInsertionResults); + checkEqualityOfInsertionResults(containerInsertionResults, worldStateInsertionResults, PUBLIC_DATA_TREE_HEIGHT); }); }); describe('Simple Nullifier Consistency', () => { const treeId = MerkleTreeId.NULLIFIER_TREE as IndexedTreeId; - let containerInsertionResults: IndexedInsertionResult[] = []; - let worldStateInsertionResults: BatchInsertionResult[] = []; + let containerInsertionResults: IndexedInsertResult[] = []; + let worldStateInsertionResults: SequentialInsertionResult[] = []; // We need to zero out between tests afterEach(() => { @@ -297,7 +304,7 @@ describe('Simple Nullifier Consistency', () => { expect(computedRoot.toBuffer()).toEqual(wsRoot); // Check that all the accumulated insertion results match - checkEqualityOfInsertionResults(containerInsertionResults, worldStateInsertionResults); + checkEqualityOfInsertionResults(containerInsertionResults, worldStateInsertionResults, NULLIFIER_TREE_HEIGHT); // Check a sibling path from a random index is consistent const wsSiblingPath = await getWorldStateSiblingPath(treeId, getSiblingIndex); @@ -325,7 +332,11 @@ describe('Simple Nullifier Consistency', () => { expect(computedRoot.toBuffer()).toEqual(wsRoot); // Check insertion results - note we can only compare against the post-insertion results - checkEqualityOfInsertionResults(containerInsertionResults, worldStateInsertionResults.slice(preInsertIndex)); + checkEqualityOfInsertionResults( + containerInsertionResults, + worldStateInsertionResults.slice(preInsertIndex), + NULLIFIER_TREE_HEIGHT, + ); }); it('Should check that the insertion paths resolve to the root', async () => { @@ -366,7 +377,7 @@ describe('Simple Nullifier Consistency', () => { // Update low nullifier const newLowNullifier = containerInsert.lowWitness.preimage; newLowNullifier.nextIndex = containerInsert.leafIndex; - newLowNullifier.nextNullifier = containerInsert.newOrElementToUpdate.element.nullifier; + newLowNullifier.nextNullifier = containerInsert.element.nullifier; // Compute new root const updatedRoot = calcRootFromPath( containerInsert.lowWitness.siblingPath, @@ -381,7 +392,7 @@ describe('Simple Nullifier Consistency', () => { // Step 4 const finalRoot = calcRootFromPath( containerInsert.insertionPath, - treeContainer.hashPreimage(containerInsert.newOrElementToUpdate.element), + treeContainer.hashPreimage(containerInsert.element), containerInsert.leafIndex, ); expect(finalRoot.toBuffer()).toEqual(rootAfter); @@ -410,7 +421,7 @@ describe('Big Random Avm Ephemeral Container Test', () => { // Insert values ino merkleTrees // Note Hash - await worldStateTrees.appendLeaves(MerkleTreeId.NOTE_HASH_TREE, noteHashes.slice(0, ENTRY_COUNT)); + await mainState.appendLeaves(MerkleTreeId.NOTE_HASH_TREE, noteHashes.slice(0, ENTRY_COUNT)); // Everything else for (let i = 0; i < ENTRY_COUNT; i++) { await nullifierInsertWorldState(indexedHashes[i]); @@ -477,7 +488,7 @@ describe('Checking forking and merging', () => { it('Fork-Rollback-Fork-Merge should be consistent', async () => { // To store results - const wsInsertionResults: BatchInsertionResult[] = []; + const wsInsertionResults: SequentialInsertionResult[] = []; const containerInsertionResults = []; const treeContainer = await AvmEphemeralForest.create(copyState); @@ -506,7 +517,7 @@ describe('Checking forking and merging', () => { expect(containerRoot.toBuffer()).toEqual(wsRoot); // Check that all the accumulated insertion results - checkEqualityOfInsertionResults(containerInsertionResults, wsInsertionResults); + checkEqualityOfInsertionResults(containerInsertionResults, wsInsertionResults, PUBLIC_DATA_TREE_HEIGHT); }); }); @@ -554,7 +565,7 @@ describe('Batch Insertion', () => { const treeContainer = await AvmEphemeralForest.create(copyState); await treeContainer.appendNullifier(indexedHashes[0]); await treeContainer.appendNullifier(indexedHashes[1]); - await worldStateTrees.batchInsert( + await mainState.batchInsert( MerkleTreeId.NULLIFIER_TREE, [indexedHashes[0].toBuffer(), indexedHashes[1].toBuffer()], NULLIFIER_SUBTREE_HEIGHT, diff --git a/yarn-project/simulator/src/avm/avm_tree.ts b/yarn-project/simulator/src/avm/avm_tree.ts index 86c4b160d7d..f9cc70f745e 100644 --- a/yarn-project/simulator/src/avm/avm_tree.ts +++ b/yarn-project/simulator/src/avm/avm_tree.ts @@ -17,7 +17,14 @@ import cloneDeep from 'lodash.clonedeep'; type PreimageWitness = { preimage: T; index: bigint; - update: boolean; +}; + +/** + * The result of fetching a leaf from an indexed tree. Contains the preimage and wether the leaf was already present + * or it's a low leaf. + */ +type GetLeafResult = PreimageWitness & { + alreadyPresent: boolean; }; /** @@ -29,16 +36,30 @@ type LeafWitness = PreimageWitness & { }; /** - * The result of an indexed insertion in an indexed merkle tree. + * The result of an update in an indexed merkle tree (no new leaf inserted) + */ +type IndexedUpdateResult = { + element: T; + lowWitness: LeafWitness; +}; + +/** + * The result of an insertion in an indexed merkle tree. * This will be used to hint the circuit */ -export type IndexedInsertionResult = { +export type IndexedInsertResult = IndexedUpdateResult & { leafIndex: bigint; insertionPath: Fr[]; - newOrElementToUpdate: { update: boolean; element: T }; - lowWitness: LeafWitness; }; +/** + * The result of an indexed upsert in an indexed merkle tree. + * This will be used to hint the circuit + */ +export type IndexedUpsertResult = + | (IndexedUpdateResult & { update: true }) + | (IndexedInsertResult & { update: false }); + /****************************************************/ /****** The AvmEphemeralForest Class ****************/ /****************************************************/ @@ -144,7 +165,7 @@ export class AvmEphemeralForest { * @param newValue - The value to be written or updated to * @returns The insertion result which contains the insertion path, low leaf and the new leaf index */ - async writePublicStorage(slot: Fr, newValue: Fr): Promise> { + async writePublicStorage(slot: Fr, newValue: Fr): Promise> { // This only works for the public data tree const treeId = MerkleTreeId.PUBLIC_DATA_TREE; const tree = this.treeMap.get(treeId)!; @@ -152,12 +173,12 @@ export class AvmEphemeralForest { typeof treeId, PublicDataTreeLeafPreimage >(treeId, slot); - const { preimage, index, update } = leafOrLowLeafInfo; - const siblingPath = await this.getSiblingPath(treeId, index); + const { preimage, index: lowLeafIndex, alreadyPresent: update } = leafOrLowLeafInfo; + const siblingPath = await this.getSiblingPath(treeId, lowLeafIndex); if (pathAbsentInEphemeralTree) { // Since we have never seen this before - we should insert it into our tree as it is about to be updated. - this.treeMap.get(treeId)!.insertSiblingPath(index, siblingPath); + this.treeMap.get(treeId)!.insertSiblingPath(lowLeafIndex, siblingPath); } if (update) { @@ -165,29 +186,18 @@ export class AvmEphemeralForest { const existingPublicDataSiblingPath = siblingPath; updatedPreimage.value = newValue; - // It is really unintuitive that by updating, we are also appending a Zero Leaf to the tree - // Additionally, this leaf preimage does not seem to factor into further appends - const emptyLeaf = new PublicDataTreeLeafPreimage(Fr.ZERO, Fr.ZERO, Fr.ZERO, 0n); - const insertionIndex = tree.leafCount; - tree.updateLeaf(this.hashPreimage(updatedPreimage), index); - tree.appendLeaf(Fr.ZERO); - this.setIndexedUpdates(treeId, index, updatedPreimage); - this.setIndexedUpdates(treeId, insertionIndex, emptyLeaf); - const insertionPath = tree.getSiblingPath(insertionIndex)!; - - // Even though we append an empty leaf into the tree as a part of update - it doesnt seem to impact future inserts... - this._updateSortedKeys(treeId, [updatedPreimage.slot], [index]); + tree.updateLeaf(this.hashPreimage(updatedPreimage), lowLeafIndex); + this.setIndexedUpdates(treeId, lowLeafIndex, updatedPreimage); + this._updateSortedKeys(treeId, [updatedPreimage.slot], [lowLeafIndex]); return { - leafIndex: insertionIndex, - insertionPath, - newOrElementToUpdate: { update: true, element: updatedPreimage }, + element: updatedPreimage, lowWitness: { preimage: preimage, - index: index, - update: true, + index: lowLeafIndex, siblingPath: existingPublicDataSiblingPath, }, + update: true, }; } // We are writing to a new slot, so our preimage is a lowNullifier @@ -202,22 +212,22 @@ export class AvmEphemeralForest { new Fr(preimage.getNextKey()), preimage.getNextIndex(), ); - const insertionPath = this.appendIndexedTree(treeId, index, updatedLowLeaf, newPublicDataLeaf); + const insertionPath = this.appendIndexedTree(treeId, lowLeafIndex, updatedLowLeaf, newPublicDataLeaf); // Even though the low leaf key is not updated, we still need to update the sorted keys in case we have // not seen the low leaf before - this._updateSortedKeys(treeId, [newPublicDataLeaf.slot, updatedLowLeaf.slot], [insertionIndex, index]); + this._updateSortedKeys(treeId, [newPublicDataLeaf.slot, updatedLowLeaf.slot], [insertionIndex, lowLeafIndex]); return { - leafIndex: insertionIndex, - insertionPath: insertionPath, - newOrElementToUpdate: { update: false, element: newPublicDataLeaf }, + element: newPublicDataLeaf, lowWitness: { preimage, - index: index, - update: false, + index: lowLeafIndex, siblingPath, }, + update: false, + leafIndex: insertionIndex, + insertionPath: insertionPath, }; } @@ -247,14 +257,14 @@ export class AvmEphemeralForest { * @param value - The nullifier to be appended * @returns The insertion result which contains the insertion path, low leaf and the new leaf index */ - async appendNullifier(nullifier: Fr): Promise> { + async appendNullifier(nullifier: Fr): Promise> { const treeId = MerkleTreeId.NULLIFIER_TREE; const tree = this.treeMap.get(treeId)!; const [leafOrLowLeafInfo, pathAbsentInEphemeralTree] = await this._getLeafOrLowLeafInfo< typeof treeId, NullifierLeafPreimage >(treeId, nullifier); - const { preimage, index, update } = leafOrLowLeafInfo; + const { preimage, index, alreadyPresent } = leafOrLowLeafInfo; const siblingPath = await this.getSiblingPath(treeId, index); if (pathAbsentInEphemeralTree) { @@ -262,7 +272,7 @@ export class AvmEphemeralForest { this.treeMap.get(treeId)!.insertSiblingPath(index, siblingPath); } - assert(!update, 'Nullifier already exists in the tree. Cannot update a nullifier!'); + assert(!alreadyPresent, 'Nullifier already exists in the tree. Cannot update a nullifier!'); // We are writing a new entry const insertionIndex = tree.leafCount; @@ -282,15 +292,14 @@ export class AvmEphemeralForest { ); return { - leafIndex: insertionIndex, - insertionPath: insertionPath, - newOrElementToUpdate: { update: false, element: newNullifierLeaf }, + element: newNullifierLeaf, lowWitness: { preimage, index, - update, siblingPath, }, + leafIndex: insertionIndex, + insertionPath: insertionPath, }; } @@ -360,7 +369,7 @@ export class AvmEphemeralForest { async getLeafOrLowLeafInfo( treeId: ID, key: Fr, - ): Promise> { + ): Promise> { const [leafOrLowLeafInfo, _] = await this._getLeafOrLowLeafInfo(treeId, key); return leafOrLowLeafInfo; } @@ -373,14 +382,14 @@ export class AvmEphemeralForest { * @param key - The key for which we are look up the leaf or low leaf for. * @param T - The type of the preimage (PublicData or Nullifier) * @returns [ - * preimageWitness - The leaf or low leaf info (preimage & leaf index), + * getLeafResult - The leaf or low leaf info (preimage & leaf index), * pathAbsentInEphemeralTree - whether its sibling path is absent in the ephemeral tree (useful during insertions) * ] */ async _getLeafOrLowLeafInfo( treeId: ID, key: Fr, - ): Promise<[PreimageWitness, /*pathAbsentInEphemeralTree=*/ boolean]> { + ): Promise<[GetLeafResult, /*pathAbsentInEphemeralTree=*/ boolean]> { const bigIntKey = key.toBigInt(); // In this function, "min" refers to the leaf with the // largest key <= the specified key in the indexedUpdates. @@ -392,7 +401,7 @@ export class AvmEphemeralForest { if (minIndexedLeafIndex === -1n) { // No leaf is present in the indexed updates that is <= the key, // so retrieve the leaf or low leaf from the underlying DB. - const leafOrLowLeafPreimage: PreimageWitness = await this._getLeafOrLowLeafWitnessInExternalDb( + const leafOrLowLeafPreimage: GetLeafResult = await this._getLeafOrLowLeafWitnessInExternalDb( treeId, bigIntKey, ); @@ -402,7 +411,7 @@ export class AvmEphemeralForest { const minPreimage: T = this.getIndexedUpdate(treeId, minIndexedLeafIndex); if (minPreimage.getKey() === bigIntKey) { // the index found is an exact match, no need to search further - const leafInfo = { preimage: minPreimage, index: minIndexedLeafIndex, update: true }; + const leafInfo = { preimage: minPreimage, index: minIndexedLeafIndex, alreadyPresent: true }; return [leafInfo, /*pathAbsentInEphemeralTree=*/ false]; } else { // We are starting with the leaf with largest key <= the specified key @@ -453,7 +462,7 @@ export class AvmEphemeralForest { private async _getLeafOrLowLeafWitnessInExternalDb( treeId: ID, key: bigint, - ): Promise> { + ): Promise> { // "key" is siloed slot (leafSlot) or siloed nullifier const previousValueIndex = await this.treeDb.getPreviousValueIndex(treeId, key); assert( @@ -468,7 +477,7 @@ export class AvmEphemeralForest { `${MerkleTreeId[treeId]} low leaf preimage should never be undefined (even if target leaf does not exist)`, ); - return { preimage: leafPreimage as T, index: leafIndex, update: alreadyPresent }; + return { preimage: leafPreimage as T, index: leafIndex, alreadyPresent }; } /** @@ -481,7 +490,7 @@ export class AvmEphemeralForest { * @param minIndex - The index of the leaf with the largest key <= the specified key. * @param T - The type of the preimage (PublicData or Nullifier) * @returns [ - * preimageWitness | undefined - The leaf or low leaf info (preimage & leaf index), + * GetLeafResult | undefined - The leaf or low leaf info (preimage & leaf index), * pathAbsentInEphemeralTree - whether its sibling path is absent in the ephemeral tree (useful during insertions) * ] * @@ -497,10 +506,10 @@ export class AvmEphemeralForest { key: bigint, minPreimage: T, minIndex: bigint, - ): Promise<[PreimageWitness | undefined, /*pathAbsentInEphemeralTree=*/ boolean]> { + ): Promise<[GetLeafResult | undefined, /*pathAbsentInEphemeralTree=*/ boolean]> { let found = false; let curr = minPreimage as T; - let result: PreimageWitness | undefined = undefined; + let result: GetLeafResult | undefined = undefined; // Temp to avoid infinite loops - the limit is the number of leaves we may have to read const LIMIT = 2n ** BigInt(getTreeHeight(treeId)) - 1n; let counter = 0n; @@ -511,11 +520,11 @@ export class AvmEphemeralForest { if (curr.getKey() === bigIntKey) { // We found an exact match - therefore this is an update found = true; - result = { preimage: curr, index: lowPublicDataIndex, update: true }; + result = { preimage: curr, index: lowPublicDataIndex, alreadyPresent: true }; } else if (curr.getKey() < bigIntKey && (curr.getNextIndex() === 0n || curr.getNextKey() > bigIntKey)) { // We found it via sandwich or max condition, this is a low nullifier found = true; - result = { preimage: curr, index: lowPublicDataIndex, update: false }; + result = { preimage: curr, index: lowPublicDataIndex, alreadyPresent: false }; } // Update the the values for the next iteration else { diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts index 9a3ffa5273a..63dbf59f09e 100644 --- a/yarn-project/simulator/src/avm/journal/journal.ts +++ b/yarn-project/simulator/src/avm/journal/journal.ts @@ -163,8 +163,12 @@ export class AvmPersistableStateManager { const lowLeafIndex = lowLeafInfo.index; const lowLeafPath = lowLeafInfo.siblingPath; - const insertionPath = result.insertionPath; - const newLeafPreimage = result.newOrElementToUpdate.element as PublicDataTreeLeafPreimage; + const newLeafPreimage = result.element as PublicDataTreeLeafPreimage; + let insertionPath; + + if (!result.update) { + insertionPath = result.insertionPath; + } this.trace.tracePublicStorageWrite( contractAddress, @@ -200,7 +204,7 @@ export class AvmPersistableStateManager { const { preimage, index: leafIndex, - update: exists, + alreadyPresent, } = await this.merkleTrees.getLeafOrLowLeafInfo(MerkleTreeId.PUBLIC_DATA_TREE, leafSlot); // The index and preimage here is either the low leaf or the leaf itself (depending on the value of update flag) // In either case, we just want the sibling path to this leaf - it's up to the avm to distinguish if it's a low leaf or not @@ -212,7 +216,7 @@ export class AvmPersistableStateManager { ); this.log.debug(`leafPreimage.slot: ${leafPreimage.slot}, leafPreimage.value: ${leafPreimage.value}`); - if (!exists) { + if (!alreadyPresent) { // Sanity check that the leaf slot is skipped by low leaf when it doesn't exist assert( leafPreimage.slot.toBigInt() < leafSlot.toBigInt() && @@ -308,12 +312,15 @@ export class AvmPersistableStateManager { const { preimage, index: leafIndex, - update, + alreadyPresent, } = await this.merkleTrees.getLeafOrLowLeafInfo(MerkleTreeId.NULLIFIER_TREE, siloedNullifier); const leafPreimage = preimage as NullifierLeafPreimage; const leafPath = await this.merkleTrees.getSiblingPath(MerkleTreeId.NULLIFIER_TREE, leafIndex); - assert(update == exists, 'WorldStateDB contains nullifier leaf, but merkle tree does not.... This is a bug!'); + assert( + alreadyPresent == exists, + 'WorldStateDB contains nullifier leaf, but merkle tree does not.... This is a bug!', + ); if (exists) { this.log.debug(`Siloed nullifier ${siloedNullifier} exists at leafIndex=${leafIndex}`); @@ -355,11 +362,11 @@ export class AvmPersistableStateManager { // Maybe overkill, but we should check if the nullifier is already present in the tree before attempting to insert // It might be better to catch the error from the insert operation // Trace all nullifier creations, even duplicate insertions that fail - const { preimage, index, update } = await this.merkleTrees.getLeafOrLowLeafInfo( + const { preimage, index, alreadyPresent } = await this.merkleTrees.getLeafOrLowLeafInfo( MerkleTreeId.NULLIFIER_TREE, siloedNullifier, ); - if (update) { + if (alreadyPresent) { this.log.verbose(`Siloed nullifier ${siloedNullifier} already present in tree at index ${index}!`); // If the nullifier is already present, we should not insert it again // instead we provide the direct membership path @@ -367,7 +374,7 @@ export class AvmPersistableStateManager { // This just becomes a nullifier read hint this.trace.traceNullifierCheck( siloedNullifier, - /*exists=*/ update, + /*exists=*/ alreadyPresent, preimage as NullifierLeafPreimage, new Fr(index), path, From 6b4d5866e11a08dbacea27e05af867aa2af23e1e Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Dec 2024 10:55:15 +0000 Subject: [PATCH 2/6] fix public_processor --- yarn-project/prover-client/src/mocks/fixtures.ts | 3 +-- yarn-project/prover-node/src/job/epoch-proving-job.ts | 3 +-- yarn-project/simulator/src/public/public_processor.ts | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/yarn-project/prover-client/src/mocks/fixtures.ts b/yarn-project/prover-client/src/mocks/fixtures.ts index 34b7cee5935..41d39929225 100644 --- a/yarn-project/prover-client/src/mocks/fixtures.ts +++ b/yarn-project/prover-client/src/mocks/fixtures.ts @@ -109,10 +109,9 @@ export const updateExpectedTreesFromTxs = async (db: MerkleTreeWriteOperations, NULLIFIER_TREE_HEIGHT, ); for (const tx of txs) { - await db.batchInsert( + await db.sequentialInsert( MerkleTreeId.PUBLIC_DATA_TREE, tx.txEffect.publicDataWrites.map(write => write.toBuffer()), - 0, ); } }; diff --git a/yarn-project/prover-node/src/job/epoch-proving-job.ts b/yarn-project/prover-node/src/job/epoch-proving-job.ts index c50e6682be4..2f6d62f573f 100644 --- a/yarn-project/prover-node/src/job/epoch-proving-job.ts +++ b/yarn-project/prover-node/src/job/epoch-proving-job.ts @@ -198,10 +198,9 @@ export class EpochProvingJob { .filter(write => !write.isEmpty()) .map(({ leafSlot, value }) => new PublicDataTreeLeaf(leafSlot, value)); - await this.db.batchInsert( + await this.db.sequentialInsert( MerkleTreeId.PUBLIC_DATA_TREE, allPublicDataWrites.map(x => x.toBuffer()), - 0, ); } } diff --git a/yarn-project/simulator/src/public/public_processor.ts b/yarn-project/simulator/src/public/public_processor.ts index d6ba69c8302..d11ac645e59 100644 --- a/yarn-project/simulator/src/public/public_processor.ts +++ b/yarn-project/simulator/src/public/public_processor.ts @@ -166,10 +166,9 @@ export class PublicProcessor { } } - await this.db.batchInsert( + await this.db.sequentialInsert( MerkleTreeId.PUBLIC_DATA_TREE, processedTx.txEffect.publicDataWrites.map(x => x.toBuffer()), - 0, ); result.push(processedTx); returns = returns.concat(returnValues ?? []); From 5441d825c5468daca5fed8f98ce249cd1be4463a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Dec 2024 11:17:57 +0000 Subject: [PATCH 3/6] bump simulator test timeout --- yarn-project/simulator/src/avm/avm_tree.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/simulator/src/avm/avm_tree.test.ts b/yarn-project/simulator/src/avm/avm_tree.test.ts index 47f88232828..b30ef226cbb 100644 --- a/yarn-project/simulator/src/avm/avm_tree.test.ts +++ b/yarn-project/simulator/src/avm/avm_tree.test.ts @@ -97,7 +97,7 @@ beforeEach(async () => { slots = Array.from({ length: 64 }, (_, i) => new Fr(i + 128)); values = Array.from({ length: 64 }, (_, i) => new Fr(i + 256)); -}); +}, 10_000); /****************************************************/ /*************** Test Cases *************************/ From b1b6ce47c005b04627eadb6b179486b39a71d9fb Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Dec 2024 14:37:51 +0000 Subject: [PATCH 4/6] update public data tests to use sequential insertions --- .../content_addressed_indexed_tree.test.cpp | 276 ++++++++++++++---- 1 file changed, 221 insertions(+), 55 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.test.cpp index 5177b370833..651960008c2 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.test.cpp @@ -358,6 +358,20 @@ void add_value(TypeOfTree& tree, const LeafValueType& value, bool expectedSucces signal.wait_for_level(); } +template +void add_value_sequentially(TypeOfTree& tree, const LeafValueType& value, bool expectedSuccess = true) +{ + std::vector values = { value }; + Signal signal; + auto completion = [&](const TypedResponse>& response) -> void { + EXPECT_EQ(response.success, expectedSuccess); + signal.signal_level(); + }; + + tree.add_or_update_values_sequentially(values, completion); + signal.wait_for_level(); +} + template void add_values(TypeOfTree& tree, const std::vector& values, bool expectedSuccess = true) { @@ -1125,6 +1139,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, sequential_insert_allows_multip add_values_sequentially(tree, values); EXPECT_EQ(get_leaf(tree, 2).value, values[1]); + check_size(tree, 3); } template fr hash_leaf(const IndexedLeaf& leaf) @@ -1560,6 +1575,165 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_indexed_memory_with_public check_sibling_path(tree, 7, expected); } +TEST_F(PersistedContentAddressedIndexedTreeTest, test_indexed_memory_with_sequential_public_data_writes) +{ + index_t current_size = 2; + ThreadPoolPtr workers = make_thread_pool(8); + // Create a depth-3 indexed merkle tree + constexpr size_t depth = 3; + std::string name = random_string(); + LMDBTreeStore::SharedPtr db = std::make_shared(_directory, name, _mapSize, _maxReaders); + std::unique_ptr> store = + std::make_unique>(name, depth, db); + auto tree = ContentAddressedIndexedTree, Poseidon2HashPolicy>( + std::move(store), workers, current_size); + + /** + * Intial state: + * + * index 0 1 2 3 4 5 6 7 + * --------------------------------------------------------------------- + * slot 0 1 0 0 0 0 0 0 + * val 0 0 0 0 0 0 0 0 + * nextIdx 1 0 0 0 0 0 0 0 + * nextVal 1 0 0 0 0 0 0 0 + */ + IndexedPublicDataLeafType zero_leaf = create_indexed_public_data_leaf(0, 0, 1, 1); + IndexedPublicDataLeafType one_leaf = create_indexed_public_data_leaf(1, 0, 0, 0); + check_size(tree, current_size); + EXPECT_EQ(get_leaf(tree, 0), zero_leaf); + EXPECT_EQ(get_leaf(tree, 1), one_leaf); + + /** + * Add new slot:value 30:5: + * + * index 0 1 2 3 4 5 6 7 + * --------------------------------------------------------------------- + * slot 0 1 30 0 0 0 0 0 + * val 0 0 5 0 0 0 0 0 + * nextIdx 1 2 0 0 0 0 0 0 + * nextVal 1 30 0 0 0 0 0 0 + */ + add_value_sequentially(tree, PublicDataLeafValue(30, 5)); + check_size(tree, ++current_size); + EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); + EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 2, 30)); + EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 5, 0, 0)); + + /** + * Add new slot:value 10:20: + * + * index 0 1 2 3 4 5 6 7 + * --------------------------------------------------------------------- + * slot 0 1 30 10 0 0 0 0 + * val 0 0 5 20 0 0 0 0 + * nextIdx 1 3 0 2 0 0 0 0 + * nextVal 1 10 0 30 0 0 0 0 + */ + add_value_sequentially(tree, PublicDataLeafValue(10, 20)); + check_size(tree, ++current_size); + EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); + EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); + EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 5, 0, 0)); + EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); + + /** + * Update value at slot 30 to 6: + * + * index 0 1 2 3 4 5 6 7 + * --------------------------------------------------------------------- + * slot 0 1 30 10 0 0 0 0 + * val 0 0 6 20 0 0 0 0 + * nextIdx 1 3 0 2 0 0 0 0 + * nextVal 1 10 0 30 0 0 0 0 + */ + add_value_sequentially(tree, PublicDataLeafValue(30, 6)); + // The size does not increase since sequential insertion doesn't pad + check_size(tree, current_size); + + EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); + EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); + EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 0, 0)); + EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); + + /** + * Add new value slot:value 50:8: + * + * index 0 1 2 3 4 5 6 7 + * --------------------------------------------------------------------- + * slot 0 1 30 10 50 0 0 0 + * val 0 0 6 20 8 0 0 0 + * nextIdx 1 3 4 2 0 0 0 0 + * nextVal 1 10 50 30 0 0 0 0 + */ + add_value_sequentially(tree, PublicDataLeafValue(50, 8)); + check_size(tree, ++current_size); + EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); + EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); + EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 4, 50)); + EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); + EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(50, 8, 0, 0)); + + // Manually compute the node values + auto e000 = hash_leaf(get_leaf(tree, 0)); + auto e001 = hash_leaf(get_leaf(tree, 1)); + auto e010 = hash_leaf(get_leaf(tree, 2)); + auto e011 = hash_leaf(get_leaf(tree, 3)); + auto e100 = hash_leaf(get_leaf(tree, 4)); + auto e101 = fr::zero(); + auto e110 = fr::zero(); + auto e111 = fr::zero(); + + auto e00 = HashPolicy::hash_pair(e000, e001); + auto e01 = HashPolicy::hash_pair(e010, e011); + auto e10 = HashPolicy::hash_pair(e100, e101); + auto e11 = HashPolicy::hash_pair(e110, e111); + + auto e0 = HashPolicy::hash_pair(e00, e01); + auto e1 = HashPolicy::hash_pair(e10, e11); + auto root = HashPolicy::hash_pair(e0, e1); + + fr_sibling_path expected = { + e001, + e01, + e1, + }; + check_sibling_path(tree, 0, expected); + expected = { + e000, + e01, + e1, + }; + check_sibling_path(tree, 1, expected); + expected = { + e011, + e00, + e1, + }; + check_sibling_path(tree, 2, expected); + expected = { + e010, + e00, + e1, + }; + check_sibling_path(tree, 3, expected); + check_root(tree, root); + + // Check the hash path at index 6 and 7 + expected = { + e111, + e10, + e0, + }; + check_sibling_path(tree, 6, expected); + expected = { + e110, + e10, + e0, + }; + check_sibling_path(tree, 7, expected); +} + TEST_F(PersistedContentAddressedIndexedTreeTest, returns_low_leaves) { // Create a depth-8 indexed merkle tree @@ -1706,7 +1880,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_historical_leaves) * nextIdx 1 2 0 0 0 0 0 0 * nextVal 1 30 0 0 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 5)); + add_value_sequentially(tree, PublicDataLeafValue(30, 5)); commit_tree(tree); check_size(tree, ++current_size); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); @@ -1725,7 +1899,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_historical_leaves) * nextIdx 1 3 0 2 0 0 0 0 * nextVal 1 10 0 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(10, 20)); + add_value_sequentially(tree, PublicDataLeafValue(10, 20)); check_size(tree, ++current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); @@ -1750,15 +1924,14 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_historical_leaves) * nextIdx 1 3 0 2 0 0 0 0 * nextVal 1 10 0 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 6)); - // The size still increases as we pad with an empty leaf - check_size(tree, ++current_size); + add_value_sequentially(tree, PublicDataLeafValue(30, 6)); + // The size does not increase since sequential insertion doesn't pad + check_size(tree, current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 0, 0)); EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); - EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(0, 0, 0, 0)); auto leaf2AtBlock3 = PublicDataLeafValue(30, 6); check_historic_leaf(tree, leaf2AtBlock2, 2, 2, true); @@ -1772,20 +1945,19 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_historical_leaves) * * index 0 1 2 3 4 5 6 7 * --------------------------------------------------------------------- - * slot 0 1 30 10 0 50 0 0 - * val 0 0 6 20 0 8 0 0 - * nextIdx 1 3 5 2 0 0 0 0 + * slot 0 1 30 10 50 0 0 0 + * val 0 0 6 20 8 0 0 0 + * nextIdx 1 3 4 2 0 0 0 0 * nextVal 1 10 50 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(50, 8)); + add_value_sequentially(tree, PublicDataLeafValue(50, 8)); check_size(tree, ++current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); - EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 5, 50)); + EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 4, 50)); EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); - EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(0, 0, 0, 0)); - EXPECT_EQ(get_leaf(tree, 5), create_indexed_public_data_leaf(50, 8, 0, 0)); + EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(50, 8, 0, 0)); check_historic_leaf(tree, leaf2AtBlock3, 2, 3, true); @@ -2005,7 +2177,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_remove_historical_blocks) * nextIdx 1 2 0 0 0 0 0 0 * nextVal 1 30 0 0 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 5)); + add_value_sequentially(tree, PublicDataLeafValue(30, 5)); commit_tree(tree); check_size(tree, ++current_size); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); @@ -2025,7 +2197,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_remove_historical_blocks) * nextIdx 1 3 0 2 0 0 0 0 * nextVal 1 10 0 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(10, 20)); + add_value_sequentially(tree, PublicDataLeafValue(10, 20)); check_size(tree, ++current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); @@ -2052,15 +2224,13 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_remove_historical_blocks) * nextIdx 1 3 0 2 0 0 0 0 * nextVal 1 10 0 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 6)); - // The size still increases as we pad with an empty leaf - check_size(tree, ++current_size); + add_value_sequentially(tree, PublicDataLeafValue(30, 6)); + check_size(tree, current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 0, 0)); EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); - EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(0, 0, 0, 0)); check_block_and_size_data(db, 3, current_size, true); @@ -2076,20 +2246,19 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_remove_historical_blocks) * * index 0 1 2 3 4 5 6 7 * --------------------------------------------------------------------- - * slot 0 1 30 10 0 50 0 0 - * val 0 0 6 20 0 8 0 0 - * nextIdx 1 3 5 2 0 0 0 0 + * slot 0 1 30 10 50 0 0 0 + * val 0 0 6 20 8 0 0 0 + * nextIdx 1 3 4 2 0 0 0 0 * nextVal 1 10 50 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(50, 8)); + add_value_sequentially(tree, PublicDataLeafValue(50, 8)); check_size(tree, ++current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); - EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 5, 50)); + EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 4, 50)); EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); - EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(0, 0, 0, 0)); - EXPECT_EQ(get_leaf(tree, 5), create_indexed_public_data_leaf(50, 8, 0, 0)); + EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(50, 8, 0, 0)); check_block_and_size_data(db, 4, current_size, true); @@ -2175,7 +2344,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) * nextIdx 1 2 0 0 0 0 0 0 * nextVal 1 30 0 0 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 5)); + add_value_sequentially(tree, PublicDataLeafValue(30, 5)); commit_tree(tree); check_size(tree, ++current_size); @@ -2205,7 +2374,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) * nextIdx 1 3 0 2 0 0 0 0 * nextVal 1 10 0 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(10, 20)); + add_value_sequentially(tree, PublicDataLeafValue(10, 20)); check_size(tree, ++current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); @@ -2241,15 +2410,13 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) * nextIdx 1 3 0 2 0 0 0 0 * nextVal 1 10 0 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 6)); - // The size still increases as we pad with an empty leaf - check_size(tree, ++current_size); + add_value_sequentially(tree, PublicDataLeafValue(30, 6)); + check_size(tree, current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 0, 0)); EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); - EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(0, 0, 0, 0)); // All historical pre-images should be present check_leaf_by_hash(db, zero_leaf, true); @@ -2277,22 +2444,21 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) * * index 0 1 2 3 4 5 6 7 * --------------------------------------------------------------------- - * slot 0 1 30 10 0 50 0 0 - * val 0 0 6 20 0 8 0 0 - * nextIdx 1 3 5 2 0 0 0 0 + * slot 0 1 30 10 50 0 0 0 + * val 0 0 6 20 8 0 0 0 + * nextIdx 1 3 4 2 0 0 0 0 * nextVal 1 10 50 30 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(50, 8)); + add_value_sequentially(tree, PublicDataLeafValue(50, 8)); check_size(tree, ++current_size); commit_tree(tree); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 3, 10)); - EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 5, 50)); + EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 4, 50)); EXPECT_EQ(get_leaf(tree, 3), create_indexed_public_data_leaf(10, 20, 2, 30)); - EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(0, 0, 0, 0)); - EXPECT_EQ(get_leaf(tree, 5), create_indexed_public_data_leaf(50, 8, 0, 0)); + EXPECT_EQ(get_leaf(tree, 4), create_indexed_public_data_leaf(50, 8, 0, 0)); - check_indices_data(db, 50, 5, true, true); + check_indices_data(db, 50, 4, true, true); // All historical pre-images should be present check_leaf_by_hash(db, zero_leaf, true); check_leaf_by_hash(db, one_leaf, true); @@ -2301,7 +2467,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) check_leaf_by_hash(db, create_indexed_public_data_leaf(10, 20, 2, 30), true); check_leaf_by_hash(db, create_indexed_public_data_leaf(30, 6, 0, 0), true); check_leaf_by_hash(db, create_indexed_public_data_leaf(50, 8, 0, 0), true); - check_leaf_by_hash(db, create_indexed_public_data_leaf(30, 6, 5, 50), true); + check_leaf_by_hash(db, create_indexed_public_data_leaf(30, 6, 4, 50), true); check_block_and_size_data(db, 4, current_size, true); @@ -2323,8 +2489,8 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) unwind_block(tree, 4); - // Index 5 should be removed - check_indices_data(db, 50, 5, false, false); + // Index 4 should be removed + check_indices_data(db, 50, 4, false, false); // The pre-images created before block 4 should be present check_leaf_by_hash(db, zero_leaf, true); check_leaf_by_hash(db, one_leaf, true); @@ -2336,7 +2502,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) // The pre-images created in block 4 should be gone check_leaf_by_hash(db, create_indexed_public_data_leaf(50, 8, 0, 0), false); - check_leaf_by_hash(db, create_indexed_public_data_leaf(30, 6, 5, 50), false); + check_leaf_by_hash(db, create_indexed_public_data_leaf(30, 6, 4, 50), false); check_size(tree, --current_size); @@ -2346,12 +2512,12 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) // block 3 should work check_block_and_size_data(db, 3, current_size, true); - // should fail to find the leaf at index 5 - check_find_leaf_index(tree, PublicDataLeafValue(50, 8), 5, false); + // should fail to find the leaf at index 4 + check_find_leaf_index(tree, PublicDataLeafValue(50, 8), 4, false); check_find_leaf_index_from(tree, PublicDataLeafValue(50, 8), 0, 5, false); // the leaf at index 2 should no longer be as it was after block 5 - EXPECT_NE(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 5, 50)); + EXPECT_NE(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 4, 50)); // it should be as it was after block 4 EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 0, 0)); @@ -2365,7 +2531,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_blocks) check_leaf_by_hash(db, create_indexed_public_data_leaf(30, 5, 0, 0), true); check_leaf_by_hash(db, create_indexed_public_data_leaf(10, 20, 2, 30), true); - check_size(tree, --current_size); + check_size(tree, current_size); // the leaf at index 2 should no longer be as it was after block 4 EXPECT_NE(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 6, 0, 0)); @@ -2413,7 +2579,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_duplicate_block) * nextIdx 1 2 0 0 0 0 0 0 * nextVal 1 30 0 0 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 5)); + add_value_sequentially(tree, PublicDataLeafValue(30, 5)); commit_tree(tree); check_size(tree, ++current_size); fr rootAfterBlock1 = get_root(tree, false); @@ -2442,9 +2608,9 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_duplicate_block) * nextIdx 1 2 0 0 0 0 0 0 * nextVal 1 30 0 0 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 8)); + add_value_sequentially(tree, PublicDataLeafValue(30, 8)); commit_tree(tree); - check_size(tree, ++current_size); + check_size(tree, current_size); fr rootAfterBlock2 = get_root(tree, false); fr_sibling_path pathAfterBlock2 = get_sibling_path(tree, 0, false); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); @@ -2470,9 +2636,9 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_duplicate_block) * nextIdx 1 2 0 0 0 0 0 0 * nextVal 1 30 0 0 0 0 0 0 */ - add_value(tree, PublicDataLeafValue(30, 5)); + add_value_sequentially(tree, PublicDataLeafValue(30, 5)); commit_tree(tree); - check_size(tree, ++current_size); + check_size(tree, current_size); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 2, 30)); EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 5, 0, 0)); @@ -2491,7 +2657,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_duplicate_block) check_root(tree, rootAfterBlock2); check_sibling_path(tree, 0, pathAfterBlock2, false); - check_size(tree, --current_size); + check_size(tree, current_size); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 2, 30)); EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 8, 0, 0)); @@ -2510,7 +2676,7 @@ TEST_F(PersistedContentAddressedIndexedTreeTest, test_unwind_duplicate_block) check_root(tree, rootAfterBlock1); check_sibling_path(tree, 0, pathAfterBlock1, false); - check_size(tree, --current_size); + check_size(tree, current_size); EXPECT_EQ(get_leaf(tree, 0), create_indexed_public_data_leaf(0, 0, 1, 1)); EXPECT_EQ(get_leaf(tree, 1), create_indexed_public_data_leaf(1, 0, 2, 30)); EXPECT_EQ(get_leaf(tree, 2), create_indexed_public_data_leaf(30, 5, 0, 0)); From 57840a8fab3be6d9981d160fb57aeb3be7a6cd74 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Dec 2024 15:00:06 +0000 Subject: [PATCH 5/6] fix build --- .../indexed_tree/content_addressed_indexed_tree.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp index 3884c5464e4..e075e36315d 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/indexed_tree/content_addressed_indexed_tree.hpp @@ -1482,8 +1482,8 @@ void ContentAddressedIndexedTree::add_or_update_values_seq response.inner.insertion_witness_data->push_back(insertion_witness); } else { // If it's an update, append an empty witness - response.inner.insertion_witness_data->push_back(LeafUpdateWitnessData{ - .leaf = IndexedLeafValueType::empty(), .index = 0, .path = std::vector(depth_) }); + response.inner.insertion_witness_data->push_back(LeafUpdateWitnessData( + IndexedLeafValueType::empty(), 0, std::vector(depth_))); } } } From 2d54eda93f0557e06760604e5ddd463e7bc01f1f Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 2 Dec 2024 15:38:59 +0000 Subject: [PATCH 6/6] chore: update package.json --- yarn-project/foundation/package.json | 2 +- yarn-project/prover-client/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/yarn-project/foundation/package.json b/yarn-project/foundation/package.json index 149a879c05c..cdaaafa04e9 100644 --- a/yarn-project/foundation/package.json +++ b/yarn-project/foundation/package.json @@ -164,4 +164,4 @@ "engines": { "node": ">=18" } -} \ No newline at end of file +} diff --git a/yarn-project/prover-client/package.json b/yarn-project/prover-client/package.json index 65012f1a178..b8766542083 100644 --- a/yarn-project/prover-client/package.json +++ b/yarn-project/prover-client/package.json @@ -104,4 +104,4 @@ "engines": { "node": ">=18" } -} \ No newline at end of file +}