diff --git a/barretenberg/acir_tests/yarn.lock b/barretenberg/acir_tests/yarn.lock index 23cba15e717f..26596d3af033 100644 --- a/barretenberg/acir_tests/yarn.lock +++ b/barretenberg/acir_tests/yarn.lock @@ -5007,8 +5007,8 @@ __metadata: linkType: hard "tar@npm:^7.4.3": - version: 7.4.3 - resolution: "tar@npm:7.4.3" + version: 7.5.11 + resolution: "tar@npm:7.5.11" dependencies: "@isaacs/fs-minipass": "npm:^4.0.0" chownr: "npm:^3.0.0" @@ -5016,7 +5016,7 @@ __metadata: minizlib: "npm:^3.0.1" mkdirp: "npm:^3.0.1" yallist: "npm:^5.0.0" - checksum: 10c0/d4679609bb2a9b48eeaf84632b6d844128d2412b95b6de07d53d8ee8baf4ca0857c9331dfa510390a0727b550fd543d4d1a10995ad86cdf078423fbb8d99831d + checksum: 10c0/b6bb420550ef50ef23356018155e956cd83282c97b6128d8d5cfe5740c57582d806a244b2ef0bf686a74ce526babe8b8b9061527623e935e850008d86d838929 languageName: node linkType: hard diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.hpp index 0a7eaf4d4905..121244e39dd7 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.hpp @@ -60,7 +60,7 @@ template class ContentAddressedAppendOn using UnwindBlockCallback = std::function&)>; using FinalizeBlockCallback = EmptyResponseCallback; using GetBlockForIndexCallback = std::function&)>; - using CheckpointCallback = EmptyResponseCallback; + using CheckpointCallback = std::function&)>; using CheckpointCommitCallback = EmptyResponseCallback; using CheckpointRevertCallback = EmptyResponseCallback; @@ -254,8 +254,11 @@ template class ContentAddressedAppendOn void checkpoint(const CheckpointCallback& on_completion); void commit_checkpoint(const CheckpointCommitCallback& on_completion); void revert_checkpoint(const CheckpointRevertCallback& on_completion); - void commit_all_checkpoints(const CheckpointCommitCallback& on_completion); - void revert_all_checkpoints(const CheckpointRevertCallback& on_completion); + void commit_all_checkpoints_to(const CheckpointCommitCallback& on_completion); + void revert_all_checkpoints_to(const CheckpointRevertCallback& on_completion); + void commit_to_depth(uint32_t target_depth, const CheckpointCommitCallback& on_completion); + void revert_to_depth(uint32_t target_depth, const CheckpointRevertCallback& on_completion); + uint32_t checkpoint_depth() const; protected: using ReadTransaction = typename Store::ReadTransaction; @@ -1002,7 +1005,11 @@ void ContentAddressedAppendOnlyTree::rollback(const Rollba template void ContentAddressedAppendOnlyTree::checkpoint(const CheckpointCallback& on_completion) { - auto job = [=, this]() { execute_and_report([=, this]() { store_->checkpoint(); }, on_completion); }; + auto job = [=, this]() { + execute_and_report( + [=, this](TypedResponse& response) { response.inner.depth = store_->checkpoint(); }, + on_completion); + }; workers_->enqueue(job); } @@ -1023,21 +1030,46 @@ void ContentAddressedAppendOnlyTree::revert_checkpoint( } template -void ContentAddressedAppendOnlyTree::commit_all_checkpoints( +void ContentAddressedAppendOnlyTree::commit_all_checkpoints_to( const CheckpointCommitCallback& on_completion) { - auto job = [=, this]() { execute_and_report([=, this]() { store_->commit_all_checkpoints(); }, on_completion); }; + auto job = [=, this]() { execute_and_report([=, this]() { store_->commit_all_checkpoints_to(); }, on_completion); }; workers_->enqueue(job); } template -void ContentAddressedAppendOnlyTree::revert_all_checkpoints( +void ContentAddressedAppendOnlyTree::revert_all_checkpoints_to( const CheckpointRevertCallback& on_completion) { - auto job = [=, this]() { execute_and_report([=, this]() { store_->revert_all_checkpoints(); }, on_completion); }; + auto job = [=, this]() { execute_and_report([=, this]() { store_->revert_all_checkpoints_to(); }, on_completion); }; + workers_->enqueue(job); +} + +template +void ContentAddressedAppendOnlyTree::commit_to_depth( + uint32_t target_depth, const CheckpointCommitCallback& on_completion) +{ + auto job = [=, this]() { + execute_and_report([=, this]() { store_->commit_to_depth(target_depth); }, on_completion); + }; + workers_->enqueue(job); +} + +template +void ContentAddressedAppendOnlyTree::revert_to_depth( + uint32_t target_depth, const CheckpointRevertCallback& on_completion) +{ + auto job = [=, this]() { + execute_and_report([=, this]() { store_->revert_to_depth(target_depth); }, on_completion); + }; workers_->enqueue(job); } +template +uint32_t ContentAddressedAppendOnlyTree::checkpoint_depth() const +{ + return store_->checkpoint_depth(); +} template void ContentAddressedAppendOnlyTree::remove_historic_block( const block_number_t& blockNumber, const RemoveHistoricBlockCallback& on_completion) diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.test.cpp index cecff513bb46..1518b2d2bfa5 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/append_only_tree/content_addressed_append_only_tree.test.cpp @@ -2171,7 +2171,7 @@ TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_checkpoint_and_revert_fo commit_checkpoint_tree(tree, false); } -TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_commit_all_checkpoints) +TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_commit_all_checkpoints_to) { constexpr size_t depth = 10; uint32_t blockSize = 16; @@ -2223,7 +2223,7 @@ TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_commit_all_checkpoints) commit_checkpoint_tree(tree, false); } -TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_revert_all_checkpoints) +TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_revert_all_checkpoints_to) { constexpr size_t depth = 10; uint32_t blockSize = 16; @@ -2274,3 +2274,95 @@ TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_revert_all_checkpoints) revert_checkpoint_tree(tree, false); commit_checkpoint_tree(tree, false); } + +TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_commit_to_depth) +{ + constexpr size_t depth = 10; + uint32_t blockSize = 16; + std::string name = random_string(); + ThreadPoolPtr pool = make_thread_pool(1); + LMDBTreeStore::SharedPtr db = std::make_shared(_directory, name, _mapSize, _maxReaders); + + { + std::unique_ptr store = std::make_unique(name, depth, db); + TreeType tree(std::move(store), pool); + std::vector values = create_values(blockSize); + add_values(tree, values); + commit_tree(tree); + } + + std::unique_ptr store = std::make_unique(name, depth, db); + TreeType tree(std::move(store), pool); + + // Capture initial state + fr_sibling_path initial_path = get_sibling_path(tree, 0); + + // Depth 1 + checkpoint_tree(tree); + add_values(tree, create_values(blockSize)); + fr_sibling_path after_depth1_path = get_sibling_path(tree, 0); + + // Depth 2 + checkpoint_tree(tree); + add_values(tree, create_values(blockSize)); + + // Depth 3 + checkpoint_tree(tree); + add_values(tree, create_values(blockSize)); + fr_sibling_path after_depth3_path = get_sibling_path(tree, 0); + + // Commit depths 3 and 2 into depth 1, leaving depth at 1 + commit_tree_to_depth(tree, 1); + + // Data from all depths should be present + check_sibling_path(tree, 0, after_depth3_path); + + // Revert depth 1 — should go back to initial state + revert_checkpoint_tree(tree); + check_sibling_path(tree, 0, initial_path); +} + +TEST_F(PersistedContentAddressedAppendOnlyTreeTest, can_revert_to_depth) +{ + constexpr size_t depth = 10; + uint32_t blockSize = 16; + std::string name = random_string(); + ThreadPoolPtr pool = make_thread_pool(1); + LMDBTreeStore::SharedPtr db = std::make_shared(_directory, name, _mapSize, _maxReaders); + + { + std::unique_ptr store = std::make_unique(name, depth, db); + TreeType tree(std::move(store), pool); + std::vector values = create_values(blockSize); + add_values(tree, values); + commit_tree(tree); + } + + std::unique_ptr store = std::make_unique(name, depth, db); + TreeType tree(std::move(store), pool); + + // Depth 1 + checkpoint_tree(tree); + add_values(tree, create_values(blockSize)); + fr_sibling_path after_depth1_path = get_sibling_path(tree, 0); + + // Depth 2 + checkpoint_tree(tree); + add_values(tree, create_values(blockSize)); + + // Depth 3 + checkpoint_tree(tree); + add_values(tree, create_values(blockSize)); + + // Revert depths 3 and 2, leaving depth at 1 + revert_tree_to_depth(tree, 1); + + // Should be back to after depth 1 state + check_sibling_path(tree, 0, after_depth1_path); + + // Depth 1 still active — commit it + commit_checkpoint_tree(tree); + + // Should still have depth 1 data + check_sibling_path(tree, 0, after_depth1_path); +} diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp index d72d8686698f..01888967c45b 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp @@ -191,11 +191,14 @@ template class ContentAddressedCachedTreeStore { std::optional find_block_for_index(const index_t& index, ReadTransaction& tx) const; - void checkpoint(); + uint32_t checkpoint(); void revert_checkpoint(); void commit_checkpoint(); - void revert_all_checkpoints(); - void commit_all_checkpoints(); + void revert_all_checkpoints_to(); + void commit_all_checkpoints_to(); + void commit_to_depth(uint32_t depth); + void revert_to_depth(uint32_t depth); + uint32_t checkpoint_depth() const; private: using Cache = ContentAddressedCache; @@ -276,10 +279,10 @@ ContentAddressedCachedTreeStore::ContentAddressedCachedTreeStore( // These checkpoint apis modify the cache's internal state. // They acquire the mutex to prevent races with concurrent read/write operations (e.g., when C++ AVM simulation // runs on a worker thread while TypeScript calls revert_checkpoint from a timeout handler). -template void ContentAddressedCachedTreeStore::checkpoint() +template uint32_t ContentAddressedCachedTreeStore::checkpoint() { std::unique_lock lock(mtx_); - cache_.checkpoint(); + return cache_.checkpoint(); } template void ContentAddressedCachedTreeStore::revert_checkpoint() @@ -294,18 +297,36 @@ template void ContentAddressedCachedTreeStore void ContentAddressedCachedTreeStore::revert_all_checkpoints() +template void ContentAddressedCachedTreeStore::revert_all_checkpoints_to() { std::unique_lock lock(mtx_); cache_.revert_all(); } -template void ContentAddressedCachedTreeStore::commit_all_checkpoints() +template void ContentAddressedCachedTreeStore::commit_all_checkpoints_to() { std::unique_lock lock(mtx_); cache_.commit_all(); } +template void ContentAddressedCachedTreeStore::commit_to_depth(uint32_t depth) +{ + std::unique_lock lock(mtx_); + cache_.commit_to_depth(depth); +} + +template void ContentAddressedCachedTreeStore::revert_to_depth(uint32_t depth) +{ + std::unique_lock lock(mtx_); + cache_.revert_to_depth(depth); +} + +template uint32_t ContentAddressedCachedTreeStore::checkpoint_depth() const +{ + std::unique_lock lock(mtx_); + return cache_.depth(); +} + template index_t ContentAddressedCachedTreeStore::constrain_tree_size_to_only_committed( const RequestContext& requestContext, ReadTransaction& tx) const diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.hpp index 31fb0a37ae17..530d82a211a4 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.hpp @@ -47,11 +47,14 @@ template class ContentAddressedCache { ContentAddressedCache& operator=(ContentAddressedCache&& other) noexcept = default; bool operator==(const ContentAddressedCache& other) const = default; - void checkpoint(); + uint32_t checkpoint(); void revert(); void commit(); void revert_all(); void commit_all(); + void commit_to_depth(uint32_t depth); + void revert_to_depth(uint32_t depth); + uint32_t depth() const; void reset(uint32_t depth); std::pair find_low_value(const uint256_t& new_leaf_key, @@ -126,9 +129,10 @@ template ContentAddressedCache::ContentA reset(depth); } -template void ContentAddressedCache::checkpoint() +template uint32_t ContentAddressedCache::checkpoint() { journals_.emplace_back(Journal(meta_)); + return static_cast(journals_.size()); } template void ContentAddressedCache::revert() @@ -240,6 +244,31 @@ template void ContentAddressedCache::rev revert(); } } +template uint32_t ContentAddressedCache::depth() const +{ + return static_cast(journals_.size()); +} + +template void ContentAddressedCache::commit_to_depth(uint32_t target_depth) +{ + if (target_depth >= journals_.size()) { + throw std::runtime_error("Invalid depth for commit_to_depth"); + } + while (journals_.size() > target_depth) { + commit(); + } +} + +template void ContentAddressedCache::revert_to_depth(uint32_t target_depth) +{ + if (target_depth >= journals_.size()) { + throw std::runtime_error("Invalid depth for revert_to_depth"); + } + while (journals_.size() > target_depth) { + revert(); + } +} + template void ContentAddressedCache::reset(uint32_t depth) { nodes_ = std::unordered_map(); diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.test.cpp index 5e6325244a40..e690308530a0 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/content_addressed_cache.test.cpp @@ -590,3 +590,210 @@ TEST_F(ContentAddressedCacheTest, reverts_remove_all_deeper_commits_2) reverts_remove_all_deeper_commits_2(max_index, depth, num_levels); } } + +TEST_F(ContentAddressedCacheTest, checkpoint_returns_depth) +{ + CacheType cache = create_cache(40); + EXPECT_EQ(cache.depth(), 0u); + EXPECT_EQ(cache.checkpoint(), 1u); + EXPECT_EQ(cache.checkpoint(), 2u); + EXPECT_EQ(cache.checkpoint(), 3u); + EXPECT_EQ(cache.depth(), 3u); +} + +TEST_F(ContentAddressedCacheTest, depth_reports_journal_count) +{ + CacheType cache = create_cache(40); + EXPECT_EQ(cache.depth(), 0u); + cache.checkpoint(); + EXPECT_EQ(cache.depth(), 1u); + cache.checkpoint(); + EXPECT_EQ(cache.depth(), 2u); + cache.commit(); + EXPECT_EQ(cache.depth(), 1u); + cache.revert(); + EXPECT_EQ(cache.depth(), 0u); +} + +TEST_F(ContentAddressedCacheTest, commit_to_depth_partial) +{ + CacheType cache = create_cache(40); + add_to_cache(cache, 0, 100, 1000); + CacheType original_cache = cache; + + // Depth 1: base checkpoint + cache.checkpoint(); + add_to_cache(cache, 100, 100, 1000); + + // Depth 2 + cache.checkpoint(); + add_to_cache(cache, 200, 100, 1000); + + // Depth 3 + cache.checkpoint(); + add_to_cache(cache, 300, 100, 1000); + + CacheType final_cache = cache; + + // Commit down to depth 1 (commits depths 3 and 2), preserve depth 1 + cache.commit_to_depth(1); + EXPECT_EQ(cache.depth(), 1u); + + // Data from depth 2+3 is merged into depth 1's scope + EXPECT_TRUE(final_cache.is_equivalent_to(cache)); + + // Now revert depth 1 — should go back to original + cache.revert(); + EXPECT_EQ(cache.depth(), 0u); + EXPECT_TRUE(original_cache.is_equivalent_to(cache)); +} + +TEST_F(ContentAddressedCacheTest, revert_to_depth_partial) +{ + CacheType cache = create_cache(40); + add_to_cache(cache, 0, 100, 1000); + + // Depth 1: base checkpoint + cache.checkpoint(); + add_to_cache(cache, 100, 100, 1000); + CacheType after_depth1_cache = cache; + + // Depth 2 + cache.checkpoint(); + add_to_cache(cache, 200, 100, 1000); + + // Depth 3 + cache.checkpoint(); + add_to_cache(cache, 300, 100, 1000); + + // Revert down to depth 1 (reverts depths 3 and 2), preserve depth 1 + cache.revert_to_depth(1); + EXPECT_EQ(cache.depth(), 1u); + + // Data from depth 2+3 is gone, state matches after depth 1 changes + EXPECT_TRUE(after_depth1_cache.is_equivalent_to(cache)); +} + +TEST_F(ContentAddressedCacheTest, commit_to_depth_0_is_commit_all) +{ + CacheType cache = create_cache(40); + add_to_cache(cache, 0, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 100, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 200, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 300, 100, 1000); + CacheType final_cache = cache; + + cache.commit_to_depth(0); + EXPECT_EQ(cache.depth(), 0u); + EXPECT_TRUE(final_cache.is_equivalent_to(cache)); + + // No more operations possible + EXPECT_THROW(cache.commit(), std::runtime_error); + EXPECT_THROW(cache.revert(), std::runtime_error); +} + +TEST_F(ContentAddressedCacheTest, revert_to_depth_0_is_revert_all) +{ + CacheType cache = create_cache(40); + add_to_cache(cache, 0, 100, 1000); + CacheType original_cache = cache; + + cache.checkpoint(); + add_to_cache(cache, 100, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 200, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 300, 100, 1000); + + cache.revert_to_depth(0); + EXPECT_EQ(cache.depth(), 0u); + EXPECT_TRUE(original_cache.is_equivalent_to(cache)); + + EXPECT_THROW(cache.commit(), std::runtime_error); + EXPECT_THROW(cache.revert(), std::runtime_error); +} + +TEST_F(ContentAddressedCacheTest, commit_to_depth_at_current_is_single_commit) +{ + CacheType cache = create_cache(40); + add_to_cache(cache, 0, 100, 1000); + + cache.checkpoint(); + add_to_cache(cache, 100, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 200, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 300, 100, 1000); + CacheType final_cache = cache; + + // Commit only the top checkpoint (depth 3), leaving depth at 2 + EXPECT_EQ(cache.depth(), 3u); + cache.commit_to_depth(2); + EXPECT_EQ(cache.depth(), 2u); + EXPECT_TRUE(final_cache.is_equivalent_to(cache)); +} + +TEST_F(ContentAddressedCacheTest, revert_to_depth_at_current_is_single_revert) +{ + CacheType cache = create_cache(40); + add_to_cache(cache, 0, 100, 1000); + + cache.checkpoint(); + add_to_cache(cache, 100, 100, 1000); + cache.checkpoint(); + add_to_cache(cache, 200, 100, 1000); + CacheType after_depth2_cache = cache; + + cache.checkpoint(); + add_to_cache(cache, 300, 100, 1000); + + // Revert only the top checkpoint (depth 3), leaving depth at 2 + EXPECT_EQ(cache.depth(), 3u); + cache.revert_to_depth(2); + EXPECT_EQ(cache.depth(), 2u); + EXPECT_TRUE(after_depth2_cache.is_equivalent_to(cache)); +} + +TEST_F(ContentAddressedCacheTest, revert_to_depth_preserves_lower_data) +{ + CacheType cache = create_cache(40); + add_to_cache(cache, 0, 100, 1000); + CacheType original_cache = cache; + + // Depth 1 + cache.checkpoint(); + add_to_cache(cache, 100, 100, 1000); + CacheType after_depth1_cache = cache; + + // Depth 2 + cache.checkpoint(); + add_to_cache(cache, 200, 100, 1000); + + // Revert depth 2 only, leaving depth at 1 + EXPECT_EQ(cache.depth(), 2u); + cache.revert_to_depth(1); + EXPECT_EQ(cache.depth(), 1u); + EXPECT_TRUE(after_depth1_cache.is_equivalent_to(cache)); + + // Commit depth 1 — depth 1 data persists + cache.commit(); + EXPECT_EQ(cache.depth(), 0u); + EXPECT_TRUE(after_depth1_cache.is_equivalent_to(cache)); +} + +TEST_F(ContentAddressedCacheTest, commit_to_depth_invalid_depth_throws) +{ + CacheType cache = create_cache(40); + cache.checkpoint(); + cache.checkpoint(); + EXPECT_EQ(cache.depth(), 2u); + + // target_depth >= current depth is invalid + EXPECT_THROW(cache.commit_to_depth(2), std::runtime_error); + EXPECT_THROW(cache.commit_to_depth(3), std::runtime_error); + EXPECT_THROW(cache.revert_to_depth(2), std::runtime_error); + EXPECT_THROW(cache.revert_to_depth(3), std::runtime_error); +} diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/response.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/response.hpp index 8aba60bfa249..43d619161162 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/response.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/response.hpp @@ -32,6 +32,17 @@ struct TreeMetaResponse { TreeMetaResponse& operator=(TreeMetaResponse&& other) noexcept = default; }; +struct CheckpointResponse { + uint32_t depth; + + CheckpointResponse() = default; + ~CheckpointResponse() = default; + CheckpointResponse(const CheckpointResponse& other) = default; + CheckpointResponse(CheckpointResponse&& other) noexcept = default; + CheckpointResponse& operator=(const CheckpointResponse& other) = default; + CheckpointResponse& operator=(CheckpointResponse&& other) noexcept = default; +}; + struct AddDataResponse { index_t size; fr root; diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/test_fixtures.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/test_fixtures.hpp index ad736e292900..e7a56a52848f 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/test_fixtures.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/test_fixtures.hpp @@ -257,10 +257,18 @@ template void rollback_tree(TreeType& tree) call_operation(completion); } -template void checkpoint_tree(TreeType& tree) +template uint32_t checkpoint_tree(TreeType& tree) { - auto completion = [&](auto completion) { tree.checkpoint(completion); }; - call_operation(completion); + Signal signal; + uint32_t depth = 0; + auto completion = [&](const TypedResponse& response) -> void { + EXPECT_EQ(response.success, true); + depth = response.inner.depth; + signal.signal_level(); + }; + tree.checkpoint(completion); + signal.wait_for_level(); + return depth; } template void commit_checkpoint_tree(TreeType& tree, bool expected_success = true) @@ -279,13 +287,25 @@ template void revert_checkpoint_tree(TreeType& tree, bool ex template void commit_all_tree_checkpoints(TreeType& tree, bool expected_success = true) { - auto completion = [&](auto completion) { tree.commit_all_checkpoints(completion); }; + auto completion = [&](auto completion) { tree.commit_all_checkpoints_to(completion); }; call_operation(completion, expected_success); } template void revert_all_tree_checkpoints(TreeType& tree, bool expected_success = true) { - auto completion = [&](auto completion) { tree.revert_all_checkpoints(completion); }; + auto completion = [&](auto completion) { tree.revert_all_checkpoints_to(completion); }; + call_operation(completion, expected_success); +} + +template void commit_tree_to_depth(TreeType& tree, uint32_t depth, bool expected_success = true) +{ + auto completion = [&](auto completion) { tree.commit_to_depth(depth, completion); }; + call_operation(completion, expected_success); +} + +template void revert_tree_to_depth(TreeType& tree, uint32_t depth, bool expected_success = true) +{ + auto completion = [&](auto completion) { tree.revert_to_depth(depth, completion); }; call_operation(completion, expected_success); } } // namespace bb::crypto::merkle_tree diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/avm_simulate/avm_simulate_napi.cpp b/barretenberg/cpp/src/barretenberg/nodejs_module/avm_simulate/avm_simulate_napi.cpp index 01c19f45e6b5..283f9488556e 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/avm_simulate/avm_simulate_napi.cpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/avm_simulate/avm_simulate_napi.cpp @@ -281,9 +281,8 @@ Napi::Value AvmSimulateNapi::simulate(const Napi::CallbackInfo& cb_info) **********************************************************/ auto deferred = std::make_shared(env); - // Create threaded operation that runs on a dedicated std::thread (not libuv pool). - // This prevents libuv thread pool exhaustion when callbacks need libuv threads for I/O. - auto* op = new ThreadedAsyncOperation( + // Create async operation that will run on a worker thread + auto* op = new AsyncOperation( env, deferred, [data, tsfns, logger_tsfn, ws_ptr, cancellation_token](msgpack::sbuffer& result_buffer) { // Collect all thread-safe functions including logger for cleanup auto all_tsfns = tsfns.to_vector(); @@ -327,6 +326,7 @@ Napi::Value AvmSimulateNapi::simulate(const Napi::CallbackInfo& cb_info) } }); + // Napi is now responsible for destroying this object op->Queue(); return deferred->Promise(); @@ -368,8 +368,8 @@ Napi::Value AvmSimulateNapi::simulateWithHintedDbs(const Napi::CallbackInfo& cb_ // Create a deferred promise auto deferred = std::make_shared(env); - // Create threaded operation that runs on a dedicated std::thread (not libuv pool) - auto* op = new ThreadedAsyncOperation(env, deferred, [data](msgpack::sbuffer& result_buffer) { + // Create async operation that will run on a worker thread + auto* op = new AsyncOperation(env, deferred, [data](msgpack::sbuffer& result_buffer) { try { // Deserialize inputs from msgpack avm2::AvmProvingInputs inputs; @@ -393,6 +393,7 @@ Napi::Value AvmSimulateNapi::simulateWithHintedDbs(const Napi::CallbackInfo& cb_ } }); + // Napi is now responsible for destroying this object op->Queue(); return deferred->Promise(); diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/util/async_op.hpp b/barretenberg/cpp/src/barretenberg/nodejs_module/util/async_op.hpp index 13a933cd5a81..3e29d08b5f6b 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/util/async_op.hpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/util/async_op.hpp @@ -3,7 +3,6 @@ #include "barretenberg/serialize/msgpack_impl.hpp" #include #include -#include #include namespace bb::nodejs { @@ -66,78 +65,4 @@ class AsyncOperation : public Napi::AsyncWorker { msgpack::sbuffer _result; }; -/** - * @brief Runs work on a dedicated std::thread instead of the libuv thread pool. - * - * Unlike AsyncOperation (which uses Napi::AsyncWorker and occupies a libuv thread), - * this class spawns a new OS thread for each operation. This prevents AVM simulations - * from exhausting the libuv thread pool, which would deadlock when C++ callbacks need - * to invoke JS functions that themselves require libuv threads (e.g., LMDB reads). - * - * The completion callback (resolve/reject) is posted back to the JS main thread via - * a Napi::ThreadSafeFunction, so the event loop returns immediately after launch - * and is woken up only when the work is done. - * - * Usage: `auto* op = new ThreadedAsyncOperation(env, deferred, fn); op->Queue();` - * The object self-destructs after resolving/rejecting the promise. - */ -class ThreadedAsyncOperation { - public: - ThreadedAsyncOperation(Napi::Env env, std::shared_ptr deferred, async_fn fn) - : _fn(std::move(fn)) - , _deferred(std::move(deferred)) - { - // Create a no-op JS function as the TSFN target — we use the native callback form of BlockingCall - // to resolve/reject the promise, so the JS function is never actually called directly. - auto dummy = Napi::Function::New(env, [](const Napi::CallbackInfo&) {}); - _completion_tsfn = Napi::ThreadSafeFunction::New(env, dummy, "ThreadedAsyncOpComplete", 0, 1); - } - - ThreadedAsyncOperation(const ThreadedAsyncOperation&) = delete; - ThreadedAsyncOperation& operator=(const ThreadedAsyncOperation&) = delete; - ThreadedAsyncOperation(ThreadedAsyncOperation&&) = delete; - ThreadedAsyncOperation& operator=(ThreadedAsyncOperation&&) = delete; - - ~ThreadedAsyncOperation() = default; - - void Queue() - { - std::thread([this]() { - try { - _fn(_result); - _success = true; - } catch (const std::exception& e) { - _error = e.what(); - _success = false; - } catch (...) { - _error = "Unknown exception occurred during threaded async operation"; - _success = false; - } - - // Post completion back to the JS main thread - _completion_tsfn.BlockingCall( - this, [](Napi::Env env, Napi::Function /*js_callback*/, ThreadedAsyncOperation* op) { - if (op->_success) { - auto buf = Napi::Buffer::Copy(env, op->_result.data(), op->_result.size()); - op->_deferred->Resolve(buf); - } else { - auto error = Napi::Error::New(env, op->_error); - op->_deferred->Reject(error.Value()); - } - // Release the TSFN and self-destruct - op->_completion_tsfn.Release(); - delete op; - }); - }).detach(); - } - - private: - async_fn _fn; - std::shared_ptr _deferred; - Napi::ThreadSafeFunction _completion_tsfn; - msgpack::sbuffer _result; - bool _success = false; - std::string _error; -}; - } // namespace bb::nodejs diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp index 57604598396d..2386799b19a0 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp @@ -265,11 +265,11 @@ WorldStateWrapper::WorldStateWrapper(const Napi::CallbackInfo& info) _dispatcher.register_target( WorldStateMessageType::COMMIT_ALL_CHECKPOINTS, - [this](msgpack::object& obj, msgpack::sbuffer& buffer) { return commit_all_checkpoints(obj, buffer); }); + [this](msgpack::object& obj, msgpack::sbuffer& buffer) { return commit_all_checkpoints_to(obj, buffer); }); _dispatcher.register_target( WorldStateMessageType::REVERT_ALL_CHECKPOINTS, - [this](msgpack::object& obj, msgpack::sbuffer& buffer) { return revert_all_checkpoints(obj, buffer); }); + [this](msgpack::object& obj, msgpack::sbuffer& buffer) { return revert_all_checkpoints_to(obj, buffer); }); _dispatcher.register_target( WorldStateMessageType::COPY_STORES, @@ -843,10 +843,12 @@ bool WorldStateWrapper::checkpoint(msgpack::object& obj, msgpack::sbuffer& buffe TypedMessage request; obj.convert(request); - _ws->checkpoint(request.value.forkId); + uint32_t depth = _ws->checkpoint(request.value.forkId); MsgHeader header(request.header.messageId); - messaging::TypedMessage resp_msg(WorldStateMessageType::CREATE_CHECKPOINT, header, {}); + CheckpointDepthResponse resp_value{ depth }; + messaging::TypedMessage resp_msg( + WorldStateMessageType::CREATE_CHECKPOINT, header, resp_value); msgpack::pack(buffer, resp_msg); return true; @@ -880,12 +882,12 @@ bool WorldStateWrapper::revert_checkpoint(msgpack::object& obj, msgpack::sbuffer return true; } -bool WorldStateWrapper::commit_all_checkpoints(msgpack::object& obj, msgpack::sbuffer& buffer) +bool WorldStateWrapper::commit_all_checkpoints_to(msgpack::object& obj, msgpack::sbuffer& buffer) { - TypedMessage request; + TypedMessage request; obj.convert(request); - _ws->commit_all_checkpoints(request.value.forkId); + _ws->commit_all_checkpoints_to(request.value.forkId, request.value.depth); MsgHeader header(request.header.messageId); messaging::TypedMessage resp_msg(WorldStateMessageType::COMMIT_ALL_CHECKPOINTS, header, {}); @@ -894,12 +896,12 @@ bool WorldStateWrapper::commit_all_checkpoints(msgpack::object& obj, msgpack::sb return true; } -bool WorldStateWrapper::revert_all_checkpoints(msgpack::object& obj, msgpack::sbuffer& buffer) +bool WorldStateWrapper::revert_all_checkpoints_to(msgpack::object& obj, msgpack::sbuffer& buffer) { - TypedMessage request; + TypedMessage request; obj.convert(request); - _ws->revert_all_checkpoints(request.value.forkId); + _ws->revert_all_checkpoints_to(request.value.forkId, request.value.depth); MsgHeader header(request.header.messageId); messaging::TypedMessage resp_msg(WorldStateMessageType::REVERT_ALL_CHECKPOINTS, header, {}); diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.hpp b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.hpp index 02945f8899a9..cd4f0d02e8e1 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.hpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.hpp @@ -75,8 +75,8 @@ class WorldStateWrapper : public Napi::ObjectWrap { bool checkpoint(msgpack::object& obj, msgpack::sbuffer& buffer); bool commit_checkpoint(msgpack::object& obj, msgpack::sbuffer& buffer); bool revert_checkpoint(msgpack::object& obj, msgpack::sbuffer& buffer); - bool commit_all_checkpoints(msgpack::object& obj, msgpack::sbuffer& buffer); - bool revert_all_checkpoints(msgpack::object& obj, msgpack::sbuffer& buffer); + bool commit_all_checkpoints_to(msgpack::object& obj, msgpack::sbuffer& buffer); + bool revert_all_checkpoints_to(msgpack::object& obj, msgpack::sbuffer& buffer); bool copy_stores(msgpack::object& obj, msgpack::sbuffer& buffer); }; diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp index 388cdc13f0bb..47b06c15f2bc 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp @@ -88,6 +88,17 @@ struct ForkIdOnlyRequest { MSGPACK_FIELDS(forkId); }; +struct ForkIdWithDepthRequest { + uint64_t forkId; + uint32_t depth; + MSGPACK_FIELDS(forkId, depth); +}; + +struct CheckpointDepthResponse { + uint32_t depth; + MSGPACK_FIELDS(depth); +}; + struct TreeIdAndRevisionRequest { MerkleTreeId treeId; WorldStateRevision revision; diff --git a/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp b/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp index 93ab14689978..f221e93fcf6f 100644 --- a/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp +++ b/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp @@ -1062,16 +1062,16 @@ bool WorldState::determine_if_synched(std::array& metaRespo return true; } -void WorldState::checkpoint(const uint64_t& forkId) +uint32_t WorldState::checkpoint(const uint64_t& forkId) { Fork::SharedPtr fork = retrieve_fork(forkId); Signal signal(static_cast(fork->_trees.size())); - std::array local; + std::array, NUM_TREES> local; std::mutex mtx; for (auto& [id, tree] : fork->_trees) { std::visit( [&signal, &local, id, &mtx](auto&& wrapper) { - wrapper.tree->checkpoint([&signal, &local, &mtx, id](Response& resp) { + wrapper.tree->checkpoint([&signal, &local, &mtx, id](TypedResponse& resp) { { std::lock_guard lock(mtx); local[id] = std::move(resp); @@ -1087,6 +1087,8 @@ void WorldState::checkpoint(const uint64_t& forkId) throw std::runtime_error(m.message); } } + // All trees have the same checkpoint depth; return it from the first tree's response + return local[0].inner.depth; } void WorldState::commit_checkpoint(const uint64_t& forkId) @@ -1143,7 +1145,7 @@ void WorldState::revert_checkpoint(const uint64_t& forkId) } } -void WorldState::commit_all_checkpoints(const uint64_t& forkId) +void WorldState::commit_all_checkpoints_to(const uint64_t& forkId, uint32_t depth) { Fork::SharedPtr fork = retrieve_fork(forkId); Signal signal(static_cast(fork->_trees.size())); @@ -1151,14 +1153,15 @@ void WorldState::commit_all_checkpoints(const uint64_t& forkId) std::mutex mtx; for (auto& [id, tree] : fork->_trees) { std::visit( - [&signal, &local, id, &mtx](auto&& wrapper) { - wrapper.tree->commit_all_checkpoints([&signal, &local, &mtx, id](Response& resp) { + [&signal, &local, id, &mtx, depth](auto&& wrapper) { + auto callback = [&signal, &local, &mtx, id](Response& resp) { { std::lock_guard lock(mtx); local[id] = std::move(resp); } signal.signal_decrement(); - }); + }; + wrapper.tree->commit_to_depth(depth, callback); }, tree); } @@ -1170,7 +1173,7 @@ void WorldState::commit_all_checkpoints(const uint64_t& forkId) } } -void WorldState::revert_all_checkpoints(const uint64_t& forkId) +void WorldState::revert_all_checkpoints_to(const uint64_t& forkId, uint32_t depth) { Fork::SharedPtr fork = retrieve_fork(forkId); Signal signal(static_cast(fork->_trees.size())); @@ -1178,14 +1181,15 @@ void WorldState::revert_all_checkpoints(const uint64_t& forkId) std::mutex mtx; for (auto& [id, tree] : fork->_trees) { std::visit( - [&signal, &local, id, &mtx](auto&& wrapper) { - wrapper.tree->revert_all_checkpoints([&signal, &local, &mtx, id](Response& resp) { + [&signal, &local, id, &mtx, depth](auto&& wrapper) { + auto callback = [&signal, &local, &mtx, id](Response& resp) { { std::lock_guard lock(mtx); local[id] = std::move(resp); } signal.signal_decrement(); - }); + }; + wrapper.tree->revert_to_depth(depth, callback); }, tree); } diff --git a/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp b/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp index bae021ab163f..66d045cb75e8 100644 --- a/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp +++ b/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp @@ -287,11 +287,11 @@ class WorldState { const std::vector& nullifiers, const std::vector& public_writes); - void checkpoint(const uint64_t& forkId); + uint32_t checkpoint(const uint64_t& forkId); void commit_checkpoint(const uint64_t& forkId); void revert_checkpoint(const uint64_t& forkId); - void commit_all_checkpoints(const uint64_t& forkId); - void revert_all_checkpoints(const uint64_t& forkId); + void commit_all_checkpoints_to(const uint64_t& forkId, uint32_t depth); + void revert_all_checkpoints_to(const uint64_t& forkId, uint32_t depth); private: std::shared_ptr _workers; diff --git a/barretenberg/docs/yarn.lock b/barretenberg/docs/yarn.lock index 4dd0ec62576a..fe93a537c589 100644 --- a/barretenberg/docs/yarn.lock +++ b/barretenberg/docs/yarn.lock @@ -17924,9 +17924,9 @@ tar@^6.1.11: yallist "^4.0.0" tar@^7.4.0: - version "7.4.3" - resolved "https://registry.yarnpkg.com/tar/-/tar-7.4.3.tgz#88bbe9286a3fcd900e94592cda7a22b192e80571" - integrity sha512-5S7Va8hKfV7W5U6g3aYxXmlPoZVAwUMy9AOKyF2fVuZa2UD3qZjg578OrLRt8PcNN1PleVaL/5/yYATNL0ICUw== + version "7.5.11" + resolved "https://registry.yarnpkg.com/tar/-/tar-7.5.11.tgz#1250fae45d98806b36d703b30973fa8e0a6d8868" + integrity sha512-ChjMH33/KetonMTAtpYdgUFr0tbz69Fp2v7zWxQfYZX4g5ZN2nOBXm1R2xyA+lMIKrLKIoKAwFj93jE/avX9cQ== dependencies: "@isaacs/fs-minipass" "^4.0.0" chownr "^3.0.0" diff --git a/barretenberg/ts/package-lock.json b/barretenberg/ts/package-lock.json index 42926d9439df..63d04aba70fd 100644 --- a/barretenberg/ts/package-lock.json +++ b/barretenberg/ts/package-lock.json @@ -4055,9 +4055,10 @@ } }, "node_modules/glob": { - "version": "10.4.5", - "resolved": "https://registry.npmjs.org/glob/-/glob-10.4.5.tgz", - "integrity": "sha512-7Bv8RF0k6xjo7d4A/PxYLbUCfb6c+Vpd2/mB2yRDlew7Jb5hEXiCD9ibfO7wpk8i4sevK6DFny9h7EYbM3/sHg==", + "version": "10.5.0", + "resolved": "https://registry.npmjs.org/glob/-/glob-10.5.0.tgz", + "integrity": "sha512-DfXN8DfhJ7NH3Oe7cFmu3NCu1wKbkReJ8TorzSAFbSKrlNaQSKfIzqYqVY8zlbs2NLBbWpRiU52GX2PbaBVNkg==", + "deprecated": "Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me", "dev": true, "license": "ISC", "dependencies": { diff --git a/barretenberg/ts/yarn.lock b/barretenberg/ts/yarn.lock index 711f495aceb0..8a088c70ba35 100644 --- a/barretenberg/ts/yarn.lock +++ b/barretenberg/ts/yarn.lock @@ -2819,8 +2819,8 @@ __metadata: linkType: hard "glob@npm:^10.2.2, glob@npm:^10.3.10": - version: 10.4.5 - resolution: "glob@npm:10.4.5" + version: 10.5.0 + resolution: "glob@npm:10.5.0" dependencies: foreground-child: "npm:^3.1.0" jackspeak: "npm:^3.1.2" @@ -2830,7 +2830,7 @@ __metadata: path-scurry: "npm:^1.11.1" bin: glob: dist/esm/bin.mjs - checksum: 10/698dfe11828b7efd0514cd11e573eaed26b2dff611f0400907281ce3eab0c1e56143ef9b35adc7c77ecc71fba74717b510c7c223d34ca8a98ec81777b293d4ac + checksum: 10/ab3bccfefcc0afaedbd1f480cd0c4a2c0e322eb3f0aa7ceaa31b3f00b825069f17cf0f1fc8b6f256795074b903f37c0ade37ddda6a176aa57f1c2bbfe7240653 languageName: node linkType: hard diff --git a/boxes/yarn.lock b/boxes/yarn.lock index fb0e88f1b274..64de1855c311 100644 --- a/boxes/yarn.lock +++ b/boxes/yarn.lock @@ -11404,8 +11404,8 @@ __metadata: linkType: hard "tar@npm:^7.4.3": - version: 7.4.3 - resolution: "tar@npm:7.4.3" + version: 7.5.11 + resolution: "tar@npm:7.5.11" dependencies: "@isaacs/fs-minipass": "npm:^4.0.0" chownr: "npm:^3.0.0" @@ -11413,7 +11413,7 @@ __metadata: minizlib: "npm:^3.0.1" mkdirp: "npm:^3.0.1" yallist: "npm:^5.0.0" - checksum: 10c0/d4679609bb2a9b48eeaf84632b6d844128d2412b95b6de07d53d8ee8baf4ca0857c9331dfa510390a0727b550fd543d4d1a10995ad86cdf078423fbb8d99831d + checksum: 10c0/b6bb420550ef50ef23356018155e956cd83282c97b6128d8d5cfe5740c57582d806a244b2ef0bf686a74ce526babe8b8b9061527623e935e850008d86d838929 languageName: node linkType: hard diff --git a/docs/yarn.lock b/docs/yarn.lock index cadc2c58e266..df5d944bde9e 100644 --- a/docs/yarn.lock +++ b/docs/yarn.lock @@ -23257,15 +23257,15 @@ __metadata: linkType: hard "tar@npm:^7.4.0, tar@npm:^7.4.3": - version: 7.5.1 - resolution: "tar@npm:7.5.1" + version: 7.5.11 + resolution: "tar@npm:7.5.11" dependencies: "@isaacs/fs-minipass": "npm:^4.0.0" chownr: "npm:^3.0.0" minipass: "npm:^7.1.2" minizlib: "npm:^3.1.0" yallist: "npm:^5.0.0" - checksum: 10c0/0dad0596a61586180981133b20c32cfd93c5863c5b7140d646714e6ea8ec84583b879e5dc3928a4d683be6e6109ad7ea3de1cf71986d5194f81b3a016c8858c9 + checksum: 10c0/b6bb420550ef50ef23356018155e956cd83282c97b6128d8d5cfe5740c57582d806a244b2ef0bf686a74ce526babe8b8b9061527623e935e850008d86d838929 languageName: node linkType: hard diff --git a/noir-projects/aztec-nr/aztec/src/event/event_interface.nr b/noir-projects/aztec-nr/aztec/src/event/event_interface.nr index 38ba43d6e471..2c5ccd0d40ed 100644 --- a/noir-projects/aztec-nr/aztec/src/event/event_interface.nr +++ b/noir-projects/aztec-nr/aztec/src/event/event_interface.nr @@ -36,7 +36,7 @@ pub unconstrained fn compute_private_serialized_event_commitment( event_type_id: Field, ) -> Field { let mut commitment_preimage = - BoundedVec::<_, 1 + MAX_EVENT_SERIALIZED_LEN>::from_array([randomness, event_type_id]); + BoundedVec::<_, 2 + MAX_EVENT_SERIALIZED_LEN>::from_array([randomness, event_type_id]); commitment_preimage.extend_from_bounded_vec(serialized_event); poseidon2_hash_with_separator_bounded_vec(commitment_preimage, DOM_SEP__EVENT_COMMITMENT) @@ -46,12 +46,19 @@ mod test { use crate::event::event_interface::{ compute_private_event_commitment, compute_private_serialized_event_commitment, EventInterface, }; + use crate::messages::logs::event::MAX_EVENT_SERIALIZED_LEN; use crate::protocol::traits::{Serialize, ToField}; use crate::test::mocks::mock_event::MockEvent; global VALUE: Field = 7; global RANDOMNESS: Field = 10; + #[test] + unconstrained fn max_size_serialized_event_commitment() { + let serialized_event = BoundedVec::from_array([0; MAX_EVENT_SERIALIZED_LEN]); + let _ = compute_private_serialized_event_commitment(serialized_event, 0, 0); + } + #[test] unconstrained fn event_commitment_equivalence() { let event = MockEvent::new(VALUE).build_event(); diff --git a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr index b7b27b9c3d16..7f76e04954dd 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr @@ -183,34 +183,50 @@ comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() - // unpack function on it. let expected_len = <$typ as $crate::protocol::traits::Packable>::N; let actual_len = packed_note.len(); - assert( - actual_len == expected_len, - f"Expected packed note of length {expected_len} but got {actual_len} for note type id {note_type_id}" - ); - - let note = $unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); - - let note_hash = $compute_note_hash(note, owner, storage_slot, randomness); - - // The message discovery process finds settled notes, that is, notes that were created in prior transactions and are therefore already part of the note hash tree. We therefore compute the nullification note hash by treating the note as a settled note with the provided note nonce. - let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification( - aztec::note::HintedNote{ + if actual_len != expected_len { + aztec::protocol::logging::warn_log_format( + "[aztec-nr] Packed note length mismatch for note type id {2}: expected {0} fields, got {1}. Skipping note.", + [expected_len as Field, actual_len as Field, note_type_id], + ); + Option::none() + } else { + let note = $unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); + + let note_hash = $compute_note_hash(note, owner, storage_slot, randomness); + + // The message discovery process finds settled notes, that is, notes that were created in + // prior transactions and are therefore already part of the note hash tree. We therefore + // compute the nullification note hash by treating the note as a settled note with the + // provided note nonce. + let note_hash_for_nullification = + aztec::note::utils::compute_note_hash_for_nullification( + aztec::note::HintedNote { + note, + contract_address, + owner, + randomness, + storage_slot, + metadata: + aztec::note::note_metadata::SettledNoteMetadata::new( + note_nonce, + ) + .into(), + }, + ); + + let inner_nullifier = $compute_nullifier_unconstrained( note, - contract_address, owner, - randomness, - storage_slot, - metadata: aztec::note::note_metadata::SettledNoteMetadata::new(note_nonce).into() - } - ); - - let inner_nullifier = $compute_nullifier_unconstrained(note, owner, note_hash_for_nullification); - - Option::some( - aztec::messages::discovery::NoteHashAndNullifier { - note_hash, inner_nullifier - } - ) + note_hash_for_nullification, + ); + + Option::some( + aztec::messages::discovery::NoteHashAndNullifier { + note_hash, + inner_nullifier, + }, + ) + } } }, ); diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr index 312327417e48..bd5abdf3e311 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr @@ -36,25 +36,29 @@ pub struct NoteHashAndNullifier { /// ``` /// |packed_note, owner, storage_slot, note_type_id, contract_address, randomness, note_nonce| { /// if note_type_id == MyNoteType::get_id() { -/// assert(packed_note.len() == MY_NOTE_TYPE_SERIALIZATION_LENGTH); +/// if packed_note.len() != MY_NOTE_TYPE_SERIALIZATION_LENGTH { +/// Option::none() +/// } else { +/// let note = MyNoteType::unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); /// -/// let note = MyNoteType::unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); +/// let note_hash = note.compute_note_hash(owner, storage_slot, randomness); +/// let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification( +/// HintedNote { +/// note, contract_address, owner, randomness, storage_slot, +/// metadata: SettledNoteMetadata::new(note_nonce).into(), +/// }, +/// ); /// -/// let note_hash = note.compute_note_hash(owner, storage_slot, randomness); -/// let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification( -/// HintedNote{ note, contract_address, metadata: SettledNoteMetadata::new(note_nonce).into() }, -/// storage_slot -/// ); +/// let inner_nullifier = note.compute_nullifier_unconstrained(owner, note_hash_for_nullification); /// -/// let inner_nullifier = note.compute_nullifier_unconstrained(owner, note_hash_for_nullification); -/// -/// Option::some( -/// aztec::messages::discovery::NoteHashAndNullifier { -/// note_hash, inner_nullifier -/// } -/// ) +/// Option::some( +/// aztec::messages::discovery::NoteHashAndNullifier { +/// note_hash, inner_nullifier +/// } +/// ) +/// } /// } else if note_type_id == MyOtherNoteType::get_id() { -/// ... // Similar to above but calling MyOtherNoteType::unpack_content +/// ... // Similar to above but calling MyOtherNoteType::unpack /// } else { /// Option::none() // Unknown note type ID /// }; diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr index df29197a0020..77af6845e427 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr @@ -42,27 +42,37 @@ pub unconstrained fn process_partial_note_private_msg( recipient: AztecAddress, msg_metadata: u64, msg_content: BoundedVec, + tx_hash: Field, ) { - // We store the information of the partial note we found in a persistent capsule in PXE, so that we can later - // search for the public log that will complete it. - let (owner, storage_slot, randomness, note_completion_log_tag, note_type_id, packed_private_note_content) = - decode_partial_note_private_message(msg_metadata, msg_content); - - let pending = DeliveredPendingPartialNote { - owner, - storage_slot, - randomness, - note_completion_log_tag, - note_type_id, - packed_private_note_content, - recipient, - }; - - CapsuleArray::at( - contract_address, - DELIVERED_PENDING_PARTIAL_NOTE_ARRAY_LENGTH_CAPSULES_SLOT, - ) - .push(pending); + let decoded = decode_partial_note_private_message(msg_metadata, msg_content); + + if decoded.is_some() { + // We store the information of the partial note we found in a persistent capsule in PXE, so that we can later + // search for the public log that will complete it. + let (owner, storage_slot, randomness, note_completion_log_tag, note_type_id, packed_private_note_content) = + decoded.unwrap(); + + let pending = DeliveredPendingPartialNote { + owner, + storage_slot, + randomness, + note_completion_log_tag, + note_type_id, + packed_private_note_content, + recipient, + }; + + CapsuleArray::at( + contract_address, + DELIVERED_PENDING_PARTIAL_NOTE_ARRAY_LENGTH_CAPSULES_SLOT, + ) + .push(pending); + } else { + debug_log_format( + "Could not decode partial note private message from tx {0}, ignoring", + [tx_hash], + ); + } } /// Searches for logs that would result in the completion of pending partial notes, ultimately resulting in the notes diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/private_events.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_events.nr index 697761f7c959..4ead09f40119 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/private_events.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_events.nr @@ -5,7 +5,7 @@ use crate::{ processing::enqueue_event_for_validation, }, }; -use crate::protocol::{address::AztecAddress, traits::ToField}; +use crate::protocol::{address::AztecAddress, logging::debug_log_format, traits::ToField}; pub unconstrained fn process_private_event_msg( contract_address: AztecAddress, @@ -14,18 +14,27 @@ pub unconstrained fn process_private_event_msg( msg_content: BoundedVec, tx_hash: Field, ) { - let (event_type_id, randomness, serialized_event) = decode_private_event_message(msg_metadata, msg_content); + let decoded = decode_private_event_message(msg_metadata, msg_content); - let event_commitment = - compute_private_serialized_event_commitment(serialized_event, randomness, event_type_id.to_field()); + if decoded.is_some() { + let (event_type_id, randomness, serialized_event) = decoded.unwrap(); - enqueue_event_for_validation( - contract_address, - event_type_id, - randomness, - serialized_event, - event_commitment, - tx_hash, - recipient, - ); + let event_commitment = + compute_private_serialized_event_commitment(serialized_event, randomness, event_type_id.to_field()); + + enqueue_event_for_validation( + contract_address, + event_type_id, + randomness, + serialized_event, + event_commitment, + tx_hash, + recipient, + ); + } else { + debug_log_format( + "Could not decode private event message from tx {0}, ignoring", + [tx_hash], + ); + } } diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr index 6366ec9a5543..3ad8567170de 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr @@ -16,22 +16,30 @@ pub unconstrained fn process_private_note_msg( msg_metadata: u64, msg_content: BoundedVec, ) { - let (note_type_id, owner, storage_slot, randomness, packed_note) = - decode_private_note_message(msg_metadata, msg_content); + let decoded = decode_private_note_message(msg_metadata, msg_content); - attempt_note_discovery( - contract_address, - tx_hash, - unique_note_hashes_in_tx, - first_nullifier_in_tx, - recipient, - compute_note_hash_and_nullifier, - owner, - storage_slot, - randomness, - note_type_id, - packed_note, - ); + if decoded.is_some() { + let (note_type_id, owner, storage_slot, randomness, packed_note) = decoded.unwrap(); + + attempt_note_discovery( + contract_address, + tx_hash, + unique_note_hashes_in_tx, + first_nullifier_in_tx, + recipient, + compute_note_hash_and_nullifier, + owner, + storage_slot, + randomness, + note_type_id, + packed_note, + ); + } else { + debug_log_format( + "Could not decode private note message from tx {0}, ignoring", + [tx_hash], + ); + } } /// Attempts discovery of a note given information about its contents and the transaction in which it is suspected the diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr index 965d7224c8a5..2ac488d053cd 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr @@ -56,41 +56,51 @@ pub unconstrained fn process_message_plaintext( // have 3 message types: private notes, partial notes and events. // We decode the message to obtain the message type id, metadata and content. - let (msg_type_id, msg_metadata, msg_content) = decode_message(message_plaintext); + let decoded = decode_message(message_plaintext); - if msg_type_id == PRIVATE_NOTE_MSG_TYPE_ID { - debug_log("Processing private note msg"); + if decoded.is_some() { + let (msg_type_id, msg_metadata, msg_content) = decoded.unwrap(); - process_private_note_msg( - contract_address, - message_context.tx_hash, - message_context.unique_note_hashes_in_tx, - message_context.first_nullifier_in_tx, - message_context.recipient, - compute_note_hash_and_nullifier, - msg_metadata, - msg_content, - ); - } else if msg_type_id == PARTIAL_NOTE_PRIVATE_MSG_TYPE_ID { - debug_log("Processing partial note private msg"); + if msg_type_id == PRIVATE_NOTE_MSG_TYPE_ID { + debug_log("Processing private note msg"); - process_partial_note_private_msg( - contract_address, - message_context.recipient, - msg_metadata, - msg_content, - ); - } else if msg_type_id == PRIVATE_EVENT_MSG_TYPE_ID { - debug_log("Processing private event msg"); + process_private_note_msg( + contract_address, + message_context.tx_hash, + message_context.unique_note_hashes_in_tx, + message_context.first_nullifier_in_tx, + message_context.recipient, + compute_note_hash_and_nullifier, + msg_metadata, + msg_content, + ); + } else if msg_type_id == PARTIAL_NOTE_PRIVATE_MSG_TYPE_ID { + debug_log("Processing partial note private msg"); - process_private_event_msg( - contract_address, - message_context.recipient, - msg_metadata, - msg_content, - message_context.tx_hash, - ); + process_partial_note_private_msg( + contract_address, + message_context.recipient, + msg_metadata, + msg_content, + message_context.tx_hash, + ); + } else if msg_type_id == PRIVATE_EVENT_MSG_TYPE_ID { + debug_log("Processing private event msg"); + + process_private_event_msg( + contract_address, + message_context.recipient, + msg_metadata, + msg_content, + message_context.tx_hash, + ); + } else { + debug_log_format("Unknown msg type id {0}", [msg_type_id as Field]); + } } else { - debug_log_format("Unknown msg type id {0}", [msg_type_id as Field]); + debug_log_format( + "Could not decode message plaintext from tx {0}, ignoring", + [message_context.tx_hash], + ); } } diff --git a/noir-projects/aztec-nr/aztec/src/messages/encoding.nr b/noir-projects/aztec-nr/aztec/src/messages/encoding.nr index dc484086cf8a..322d5dd78103 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/encoding.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/encoding.nr @@ -90,29 +90,34 @@ pub fn encode_message( /// Decodes a standard aztec-nr message, i.e. one created via `encode_message`, returning the original encoded values. /// +/// Returns `None` if the message is empty or has invalid (>128 bit) expanded metadata. +/// /// Note that `encode_message` returns a fixed size array while this function takes a `BoundedVec`: this is because /// prior to decoding the message type is unknown, and consequentially not known at compile time. If working with /// fixed-size messages, consider using `BoundedVec::from_array` to convert them. pub unconstrained fn decode_message( message: BoundedVec, -) -> (u64, u64, BoundedVec) { - assert( - message.len() >= MESSAGE_EXPANDED_METADATA_LEN, - f"Invalid message: it must have at least {MESSAGE_EXPANDED_METADATA_LEN} fields", - ); - - // If MESSAGE_EXPANDED_METADATA_LEN is changed, causing the assertion below to fail, then the destructuring of the - // message encoding below must be updated as well. - std::static_assert( - MESSAGE_EXPANDED_METADATA_LEN == 1, - "unexpected value for MESSAGE_EXPANDED_METADATA_LEN", - ); - - let msg_expanded_metadata = message.get(0); - let (msg_type_id, msg_metadata) = from_expanded_metadata(msg_expanded_metadata); - let msg_content = array::subbvec(message, MESSAGE_EXPANDED_METADATA_LEN); - - (msg_type_id, msg_metadata, msg_content) +) -> Option<(u64, u64, BoundedVec)> { + Option::some(message) + .and_then(|message| { + // If MESSAGE_EXPANDED_METADATA_LEN is changed, causing the assertion below to fail, then the destructuring + // of the + // message encoding below must be updated as well. + std::static_assert( + MESSAGE_EXPANDED_METADATA_LEN == 1, + "unexpected value for MESSAGE_EXPANDED_METADATA_LEN", + ); + if message.len() < MESSAGE_EXPANDED_METADATA_LEN { + Option::none() + } else { + Option::some(message.get(0)) + } + }) + .and_then(|msg_expanded_metadata| from_expanded_metadata(msg_expanded_metadata)) + .map(|(msg_type_id, msg_metadata)| { + let msg_content = array::subbvec(message, MESSAGE_EXPANDED_METADATA_LEN); + (msg_type_id, msg_metadata, msg_content) + }) } global U64_SHIFT_MULTIPLIER: Field = 2.pow_32(64); @@ -126,17 +131,26 @@ fn to_expanded_metadata(msg_type: u64, msg_metadata: u64) -> Field { type_field + msg_metadata_field } -fn from_expanded_metadata(input: Field) -> (u64, u64) { - input.assert_max_bit_size::<128>(); - let msg_metadata = (input as u64); - let msg_type = ((input - (msg_metadata as Field)) / U64_SHIFT_MULTIPLIER) as u64; - // Use division instead of bit shift since bit shifts are expensive in circuits - (msg_type, msg_metadata) +global TWO_POW_128: Field = 2.pow_32(128); + +/// Unpacks expanded metadata into (msg_type, msg_metadata). Returns `None` if `input >= 2^128`. +fn from_expanded_metadata(input: Field) -> Option<(u64, u64)> { + if input.lt(TWO_POW_128) { + let msg_metadata = (input as u64); + let msg_type = ((input - (msg_metadata as Field)) / U64_SHIFT_MULTIPLIER) as u64; + // Use division instead of bit shift since bit shifts are expensive in circuits + Option::some((msg_type, msg_metadata)) + } else { + Option::none() + } } mod tests { use crate::utils::array::subarray::subarray; - use super::{decode_message, encode_message, from_expanded_metadata, MAX_MESSAGE_CONTENT_LEN, to_expanded_metadata}; + use super::{ + decode_message, encode_message, from_expanded_metadata, MAX_MESSAGE_CONTENT_LEN, to_expanded_metadata, + TWO_POW_128, + }; global U64_MAX: u64 = (2.pow_32(64) - 1) as u64; global U128_MAX: Field = (2.pow_32(128) - 1); @@ -145,7 +159,7 @@ mod tests { unconstrained fn encode_decode_empty_message(msg_type: u64, msg_metadata: u64) { let encoded = encode_message(msg_type, msg_metadata, []); let (decoded_msg_type, decoded_msg_metadata, decoded_msg_content) = - decode_message(BoundedVec::from_array(encoded)); + decode_message(BoundedVec::from_array(encoded)).unwrap(); assert_eq(decoded_msg_type, msg_type); assert_eq(decoded_msg_metadata, msg_metadata); @@ -160,7 +174,7 @@ mod tests { ) { let encoded = encode_message(msg_type, msg_metadata, msg_content); let (decoded_msg_type, decoded_msg_metadata, decoded_msg_content) = - decode_message(BoundedVec::from_array(encoded)); + decode_message(BoundedVec::from_array(encoded)).unwrap(); assert_eq(decoded_msg_type, msg_type); assert_eq(decoded_msg_metadata, msg_metadata); @@ -176,7 +190,7 @@ mod tests { ) { let encoded = encode_message(msg_type, msg_metadata, msg_content); let (decoded_msg_type, decoded_msg_metadata, decoded_msg_content) = - decode_message(BoundedVec::from_array(encoded)); + decode_message(BoundedVec::from_array(encoded)).unwrap(); assert_eq(decoded_msg_type, msg_type); assert_eq(decoded_msg_metadata, msg_metadata); @@ -188,25 +202,25 @@ mod tests { unconstrained fn to_expanded_metadata_packing() { // Test case 1: All bits set let packed = to_expanded_metadata(U64_MAX, U64_MAX); - let (msg_type, msg_metadata) = from_expanded_metadata(packed); + let (msg_type, msg_metadata) = from_expanded_metadata(packed).unwrap(); assert_eq(msg_type, U64_MAX); assert_eq(msg_metadata, U64_MAX); // Test case 2: Only log type bits set let packed = to_expanded_metadata(U64_MAX, 0); - let (msg_type, msg_metadata) = from_expanded_metadata(packed); + let (msg_type, msg_metadata) = from_expanded_metadata(packed).unwrap(); assert_eq(msg_type, U64_MAX); assert_eq(msg_metadata, 0); // Test case 3: Only msg_metadata bits set let packed = to_expanded_metadata(0, U64_MAX); - let (msg_type, msg_metadata) = from_expanded_metadata(packed); + let (msg_type, msg_metadata) = from_expanded_metadata(packed).unwrap(); assert_eq(msg_type, 0); assert_eq(msg_metadata, U64_MAX); // Test case 4: No bits set let packed = to_expanded_metadata(0, 0); - let (msg_type, msg_metadata) = from_expanded_metadata(packed); + let (msg_type, msg_metadata) = from_expanded_metadata(packed).unwrap(); assert_eq(msg_type, 0); assert_eq(msg_metadata, 0); } @@ -215,25 +229,25 @@ mod tests { unconstrained fn from_expanded_metadata_packing() { // Test case 1: All bits set let input = U128_MAX as Field; - let (msg_type, msg_metadata) = from_expanded_metadata(input); + let (msg_type, msg_metadata) = from_expanded_metadata(input).unwrap(); assert_eq(msg_type, U64_MAX); assert_eq(msg_metadata, U64_MAX); // Test case 2: Only log type bits set let input = (U128_MAX - U64_MAX as Field); - let (msg_type, msg_metadata) = from_expanded_metadata(input); + let (msg_type, msg_metadata) = from_expanded_metadata(input).unwrap(); assert_eq(msg_type, U64_MAX); assert_eq(msg_metadata, 0); // Test case 3: Only msg_metadata bits set let input = U64_MAX as Field; - let (msg_type, msg_metadata) = from_expanded_metadata(input); + let (msg_type, msg_metadata) = from_expanded_metadata(input).unwrap(); assert_eq(msg_type, 0); assert_eq(msg_metadata, U64_MAX); // Test case 4: No bits set let input = 0; - let (msg_type, msg_metadata) = from_expanded_metadata(input); + let (msg_type, msg_metadata) = from_expanded_metadata(input).unwrap(); assert_eq(msg_type, 0); assert_eq(msg_metadata, 0); } @@ -241,7 +255,7 @@ mod tests { #[test] unconstrained fn to_from_expanded_metadata(original_msg_type: u64, original_msg_metadata: u64) { let packed = to_expanded_metadata(original_msg_type, original_msg_metadata); - let (unpacked_msg_type, unpacked_msg_metadata) = from_expanded_metadata(packed); + let (unpacked_msg_type, unpacked_msg_metadata) = from_expanded_metadata(packed).unwrap(); assert_eq(original_msg_type, unpacked_msg_type); assert_eq(original_msg_metadata, unpacked_msg_metadata); @@ -257,7 +271,8 @@ mod tests { } let encoded = encode_message(msg_type_id, msg_metadata, msg_content); - let (decoded_type_id, decoded_metadata, decoded_content) = decode_message(BoundedVec::from_array(encoded)); + let (decoded_type_id, decoded_metadata, decoded_content) = + decode_message(BoundedVec::from_array(encoded)).unwrap(); assert_eq(decoded_type_id, msg_type_id); assert_eq(decoded_metadata, msg_metadata); @@ -269,4 +284,15 @@ mod tests { let msg_content = [0; MAX_MESSAGE_CONTENT_LEN + 1]; let _ = encode_message(0, 0, msg_content); } + + #[test] + unconstrained fn decode_empty_message_returns_none() { + assert(decode_message(BoundedVec::new()).is_none()); + } + + #[test] + unconstrained fn decode_message_with_oversized_metadata_returns_none() { + let message = BoundedVec::from_array([TWO_POW_128]); + assert(decode_message(message).is_none()); + } } diff --git a/noir-projects/aztec-nr/aztec/src/messages/logs/event.nr b/noir-projects/aztec-nr/aztec/src/messages/logs/event.nr index fbe759248efa..101bff9a3f68 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/logs/event.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/logs/event.nr @@ -57,7 +57,8 @@ where /// Decodes the plaintext from a private event message (i.e. one of type [`PRIVATE_EVENT_MSG_TYPE_ID`]). /// -/// This plaintext is meant to have originated from [`encode_private_event_message`]. +/// Returns `None` if `msg_content` has too few fields. This plaintext is meant to have originated +/// from [`encode_private_event_message`]. /// /// Note that while [`encode_private_event_message`] returns a fixed-size array, this function takes a [`BoundedVec`] /// instead. This is because when decoding we're typically processing runtime-sized plaintexts, more specifically, @@ -65,26 +66,24 @@ where pub(crate) unconstrained fn decode_private_event_message( msg_metadata: u64, msg_content: BoundedVec, -) -> (EventSelector, Field, BoundedVec) { - // Private event messages contain the event type id in the metadata - let event_type_id = EventSelector::from_field(msg_metadata as Field); - - assert( - msg_content.len() > PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN, - f"Invalid private event message: all private event messages must have at least {PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN} fields", - ); - - // If PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN is changed, causing the assertion below to fail, then the - // destructuring of the private event message encoding below must be updated as well. - std::static_assert( - PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN == 1, - "unexpected value for PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN", - ); - - let randomness = msg_content.get(PRIVATE_EVENT_MSG_PLAINTEXT_RANDOMNESS_INDEX); - let serialized_event = array::subbvec(msg_content, PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN); - - (event_type_id, randomness, serialized_event) +) -> Option<(EventSelector, Field, BoundedVec)> { + if msg_content.len() <= PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN { + Option::none() + } else { + let event_type_id = EventSelector::from_field(msg_metadata as Field); + + // If PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN is changed, causing the assertion below to fail, then the + // destructuring of the private event message encoding below must be updated as well. + std::static_assert( + PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN == 1, + "unexpected value for PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN", + ); + + let randomness = msg_content.get(PRIVATE_EVENT_MSG_PLAINTEXT_RANDOMNESS_INDEX); + let serialized_event = array::subbvec(msg_content, PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN); + + Option::some((event_type_id, randomness, serialized_event)) + } } mod test { @@ -108,14 +107,28 @@ mod test { let message_plaintext = encode_private_event_message(event, RANDOMNESS); - let (msg_type_id, msg_metadata, msg_content) = decode_message(BoundedVec::from_array(message_plaintext)); + let (msg_type_id, msg_metadata, msg_content) = + decode_message(BoundedVec::from_array(message_plaintext)).unwrap(); assert_eq(msg_type_id, PRIVATE_EVENT_MSG_TYPE_ID); - let (event_type_id, randomness, serialized_event) = decode_private_event_message(msg_metadata, msg_content); + let (event_type_id, randomness, serialized_event) = + decode_private_event_message(msg_metadata, msg_content).unwrap(); assert_eq(event_type_id, MockEvent::get_event_type_id()); assert_eq(randomness, RANDOMNESS); assert_eq(serialized_event, BoundedVec::from_array(event.serialize())); } + + #[test] + unconstrained fn decode_empty_content_returns_none() { + let empty = BoundedVec::new(); + assert(decode_private_event_message(0, empty).is_none()); + } + + #[test] + unconstrained fn decode_with_only_reserved_fields_returns_none() { + let content = BoundedVec::from_array([0]); + assert(decode_private_event_message(0, content).is_none()); + } } diff --git a/noir-projects/aztec-nr/aztec/src/messages/logs/note.nr b/noir-projects/aztec-nr/aztec/src/messages/logs/note.nr index 84a72a48e534..b78a17b09102 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/logs/note.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/logs/note.nr @@ -54,7 +54,8 @@ where /// Decodes the plaintext from a private note message (i.e. one of type [`PRIVATE_NOTE_MSG_TYPE_ID`]). /// -/// This plaintext is meant to have originated from [`encode_private_note_message`]. +/// Returns `None` if `msg_content` has too few fields. This plaintext is meant to have originated +/// from [`encode_private_note_message`]. /// /// Note that while [`encode_private_note_message`] returns a fixed-size array, this function takes a [`BoundedVec`] /// instead. This is because when decoding we're typically processing runtime-sized plaintexts, more specifically, @@ -62,27 +63,26 @@ where pub(crate) unconstrained fn decode_private_note_message( msg_metadata: u64, msg_content: BoundedVec, -) -> (Field, AztecAddress, Field, Field, BoundedVec) { - let note_type_id = msg_metadata as Field; // TODO: make note type id not be a full field - - assert( - msg_content.len() > PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN, - f"Invalid private note message: all private note messages must have at least {PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN} fields", - ); - - // If PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN is changed, causing the assertion below to fail, then the - // decoding below must be updated as well. - std::static_assert( - PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN == 3, - "unexpected value for PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN", - ); - - let owner = AztecAddress::from_field(msg_content.get(PRIVATE_NOTE_MSG_PLAINTEXT_OWNER_INDEX)); - let storage_slot = msg_content.get(PRIVATE_NOTE_MSG_PLAINTEXT_STORAGE_SLOT_INDEX); - let randomness = msg_content.get(PRIVATE_NOTE_MSG_PLAINTEXT_RANDOMNESS_INDEX); - let packed_note = array::subbvec(msg_content, PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN); - - (note_type_id, owner, storage_slot, randomness, packed_note) +) -> Option<(Field, AztecAddress, Field, Field, BoundedVec)> { + if msg_content.len() <= PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN { + Option::none() + } else { + let note_type_id = msg_metadata as Field; // TODO: make note type id not be a full field + + // If PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN is changed, causing the assertion below to fail, then the + // decoding below must be updated as well. + std::static_assert( + PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN == 3, + "unexpected value for PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN", + ); + + let owner = AztecAddress::from_field(msg_content.get(PRIVATE_NOTE_MSG_PLAINTEXT_OWNER_INDEX)); + let storage_slot = msg_content.get(PRIVATE_NOTE_MSG_PLAINTEXT_STORAGE_SLOT_INDEX); + let randomness = msg_content.get(PRIVATE_NOTE_MSG_PLAINTEXT_RANDOMNESS_INDEX); + let packed_note = array::subbvec(msg_content, PRIVATE_NOTE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN); + + Option::some((note_type_id, owner, storage_slot, randomness, packed_note)) + } } mod test { @@ -108,12 +108,13 @@ mod test { let message_plaintext = encode_private_note_message(note, OWNER, STORAGE_SLOT, RANDOMNESS); - let (msg_type_id, msg_metadata, msg_content) = decode_message(BoundedVec::from_array(message_plaintext)); + let (msg_type_id, msg_metadata, msg_content) = + decode_message(BoundedVec::from_array(message_plaintext)).unwrap(); assert_eq(msg_type_id, PRIVATE_NOTE_MSG_TYPE_ID); let (note_type_id, owner, storage_slot, randomness, packed_note) = - decode_private_note_message(msg_metadata, msg_content); + decode_private_note_message(msg_metadata, msg_content).unwrap(); assert_eq(note_type_id, MockNote::get_id()); assert_eq(owner, OWNER); @@ -142,12 +143,12 @@ mod test { let note = MaxSizeNote { data }; let encoded = encode_private_note_message(note, OWNER, STORAGE_SLOT, RANDOMNESS); - let (msg_type_id, msg_metadata, msg_content) = decode_message(BoundedVec::from_array(encoded)); + let (msg_type_id, msg_metadata, msg_content) = decode_message(BoundedVec::from_array(encoded)).unwrap(); assert_eq(msg_type_id, PRIVATE_NOTE_MSG_TYPE_ID); let (note_type_id, owner, storage_slot, randomness, packed_note) = - decode_private_note_message(msg_metadata, msg_content); + decode_private_note_message(msg_metadata, msg_content).unwrap(); assert_eq(note_type_id, MaxSizeNote::get_id()); assert_eq(owner, OWNER); @@ -172,4 +173,16 @@ mod test { let note = OversizedNote { data: [0; MAX_NOTE_PACKED_LEN + 1] }; let _ = encode_private_note_message(note, OWNER, STORAGE_SLOT, RANDOMNESS); } + + #[test] + unconstrained fn decode_empty_content_returns_none() { + let empty = BoundedVec::new(); + assert(decode_private_note_message(0, empty).is_none()); + } + + #[test] + unconstrained fn decode_with_only_reserved_fields_returns_none() { + let content = BoundedVec::from_array([0, 0, 0]); + assert(decode_private_note_message(0, content).is_none()); + } } diff --git a/noir-projects/aztec-nr/aztec/src/messages/logs/partial_note.nr b/noir-projects/aztec-nr/aztec/src/messages/logs/partial_note.nr index b39e0a809fbc..63a19ee0cd15 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/logs/partial_note.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/logs/partial_note.nr @@ -91,9 +91,11 @@ where ) } -/// Decodes the plaintext from a private note message (i.e. one of type [`PARTIAL_NOTE_PRIVATE_MSG_TYPE_ID`]). +/// Decodes the plaintext from a partial note private message (i.e. one of type +/// [`PARTIAL_NOTE_PRIVATE_MSG_TYPE_ID`]). /// -/// This plaintext is meant to have originated from [`encode_partial_note_private_message`]. +/// Returns `None` if `msg_content` has too few fields. This plaintext is meant to have originated +/// from [`encode_partial_note_private_message`]. /// /// Note that while [`encode_partial_note_private_message`] returns a fixed-size array, this function takes a /// [`BoundedVec`] instead. This is because when decoding we're typically processing runtime-sized plaintexts, more @@ -102,39 +104,37 @@ where pub(crate) unconstrained fn decode_partial_note_private_message( msg_metadata: u64, msg_content: BoundedVec, -) -> (AztecAddress, Field, Field, Field, Field, BoundedVec) { - let note_type_id = msg_metadata as Field; // TODO: make note type id not be a full field - - // The following ensures that the message content contains at least the minimum number of fields required for a - // valid partial note private message. (Refer to the description of - // PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_NON_NOTE_FIELDS_LEN for more information about these fields.) - assert( - msg_content.len() >= PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN, - f"Invalid private note message: all partial note private messages must have at least {PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN} fields", - ); - - // If PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_NON_NOTE_FIELDS_LEN is changed, causing the assertion below to fail, then - // the destructuring of the partial note private message encoding below must be updated as well. - std::static_assert( - PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN == 4, - "unexpected value for PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_NON_NOTE_FIELDS_LEN", - ); +) -> Option<(AztecAddress, Field, Field, Field, Field, BoundedVec)> { + if msg_content.len() < PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN { + Option::none() + } else { + let note_type_id: Field = msg_metadata as Field; // TODO: make note type id not be a full field + + // If PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_NON_NOTE_FIELDS_LEN is changed, causing the assertion below to fail, + // then the destructuring of the partial note private message encoding below must be updated as well. + std::static_assert( + PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN == 4, + "unexpected value for PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_NON_NOTE_FIELDS_LEN", + ); - // We currently have four fields that are not the partial note's packed representation, which are the owner, the - // storage slot, the randomness, and the note completion log tag. - let owner = AztecAddress::from_field( - msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_OWNER_INDEX), - ); - let storage_slot = msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_STORAGE_SLOT_INDEX); - let randomness = msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RANDOMNESS_INDEX); - let note_completion_log_tag = msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_NOTE_COMPLETION_LOG_TAG_INDEX); + // We currently have four fields that are not the partial note's packed representation, which are the owner, + // the storage slot, the randomness, and the note completion log tag. + let owner = AztecAddress::from_field( + msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_OWNER_INDEX), + ); + let storage_slot = msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_STORAGE_SLOT_INDEX); + let randomness = msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RANDOMNESS_INDEX); + let note_completion_log_tag = msg_content.get(PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_NOTE_COMPLETION_LOG_TAG_INDEX); - let packed_private_note_content: BoundedVec = array::subbvec( - msg_content, - PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN, - ); + let packed_private_note_content: BoundedVec = array::subbvec( + msg_content, + PARTIAL_NOTE_PRIVATE_MSG_PLAINTEXT_RESERVED_FIELDS_LEN, + ); - (owner, storage_slot, randomness, note_completion_log_tag, note_type_id, packed_private_note_content) + Option::some(( + owner, storage_slot, randomness, note_completion_log_tag, note_type_id, packed_private_note_content, + )) + } } mod test { @@ -168,12 +168,13 @@ mod test { NOTE_COMPLETION_LOG_TAG, ); - let (msg_type_id, msg_metadata, msg_content) = decode_message(BoundedVec::from_array(message_plaintext)); + let (msg_type_id, msg_metadata, msg_content) = + decode_message(BoundedVec::from_array(message_plaintext)).unwrap(); assert_eq(msg_type_id, PARTIAL_NOTE_PRIVATE_MSG_TYPE_ID); let (owner, storage_slot, randomness, note_completion_log_tag, note_type_id, packed_note) = - decode_partial_note_private_message(msg_metadata, msg_content); + decode_partial_note_private_message(msg_metadata, msg_content).unwrap(); assert_eq(note_type_id, MockNote::get_id()); assert_eq(owner, OWNER); @@ -182,4 +183,17 @@ mod test { assert_eq(note_completion_log_tag, NOTE_COMPLETION_LOG_TAG); assert_eq(packed_note, BoundedVec::from_array(note.pack())); } + + #[test] + unconstrained fn decode_empty_content_returns_none() { + let empty = BoundedVec::new(); + assert(decode_partial_note_private_message(0, empty).is_none()); + } + + #[test] + unconstrained fn decode_succeeds_with_only_reserved_fields() { + let content = BoundedVec::from_array([0, 0, 0, 0]); + let (_, _, _, _, _, packed_note) = decode_partial_note_private_message(0, content).unwrap(); + assert_eq(packed_note.len(), 0); + } } diff --git a/noir-projects/noir-contracts/Nargo.toml b/noir-projects/noir-contracts/Nargo.toml index 7b7c76bf8bad..cb61b6c33977 100644 --- a/noir-projects/noir-contracts/Nargo.toml +++ b/noir-projects/noir-contracts/Nargo.toml @@ -48,6 +48,7 @@ members = [ "contracts/test/import_test_contract", "contracts/test/invalid_account_contract", "contracts/test/no_constructor_contract", + "contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract", "contracts/test/note_getter_contract", "contracts/test/offchain_effect_contract", "contracts/test/only_self_contract", diff --git a/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/Nargo.toml b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/Nargo.toml new file mode 100644 index 000000000000..3f96bf14515a --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "note_hash_and_nullifier_contract" +authors = [""] +compiler_version = ">=0.25.0" +type = "contract" + +[dependencies] +aztec = { path = "../../../../../aztec-nr/aztec" } diff --git a/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr new file mode 100644 index 000000000000..f077b11497ae --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr @@ -0,0 +1,37 @@ +pub mod test_note; +mod test; + +use aztec::macros::aztec; + +/// A minimal contract used to test the macro-generated `_compute_note_hash_and_nullifier` function. +#[aztec] +pub contract NoteHashAndNullifier { + use aztec::{ + messages::{ + discovery::NoteHashAndNullifier as NoteHashAndNullifierResult, + logs::note::MAX_NOTE_PACKED_LEN, + }, + protocol::address::AztecAddress, + }; + + #[contract_library_method] + pub unconstrained fn test_compute_note_hash_and_nullifier( + packed_note: BoundedVec, + owner: AztecAddress, + storage_slot: Field, + note_type_id: Field, + contract_address: AztecAddress, + randomness: Field, + note_nonce: Field, + ) -> Option { + _compute_note_hash_and_nullifier( + packed_note, + owner, + storage_slot, + note_type_id, + contract_address, + randomness, + note_nonce, + ) + } +} diff --git a/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test.nr b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test.nr new file mode 100644 index 000000000000..c20909854926 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test.nr @@ -0,0 +1,65 @@ +use crate::{NoteHashAndNullifier, test_note::{TEST_NOTE_NULLIFIER, TestNote}}; +use aztec::note::note_interface::{NoteHash, NoteType}; +use aztec::protocol::address::AztecAddress; + +#[test] +unconstrained fn returns_none_for_bad_note_length() { + // TestNote has Packable N=1, but we provide 2 fields + let packed_note = BoundedVec::from_array([42, 99]); + + let result = NoteHashAndNullifier::test_compute_note_hash_and_nullifier( + packed_note, + AztecAddress::zero(), + 0, + TestNote::get_id(), + AztecAddress::zero(), + 0, + 0, + ); + + assert(result.is_none()); +} + +#[test] +unconstrained fn returns_correct_note_hash_and_nullifier() { + // TestNote has Packable N=1 + let packed_note = BoundedVec::from_array([42]); + + let owner = AztecAddress::zero(); + let storage_slot = 0; + let randomness = 0; + + let result = NoteHashAndNullifier::test_compute_note_hash_and_nullifier( + packed_note, + owner, + storage_slot, + TestNote::get_id(), + AztecAddress::zero(), + randomness, + 1, + ); + + let note_hash_and_nullifier = result.unwrap(); + let note = TestNote { value: 42 }; + let expected_note_hash = note.compute_note_hash(owner, storage_slot, randomness); + assert_eq(note_hash_and_nullifier.note_hash, expected_note_hash); + + assert_eq(note_hash_and_nullifier.inner_nullifier.unwrap(), TEST_NOTE_NULLIFIER); +} + +#[test] +unconstrained fn returns_none_for_empty_packed_note() { + let packed_note = BoundedVec::new(); + + let result = NoteHashAndNullifier::test_compute_note_hash_and_nullifier( + packed_note, + AztecAddress::zero(), + 0, + TestNote::get_id(), + AztecAddress::zero(), + 0, + 0, + ); + + assert(result.is_none()); +} diff --git a/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test_note.nr b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test_note.nr new file mode 100644 index 000000000000..5ca3704ceab7 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test_note.nr @@ -0,0 +1,48 @@ +use aztec::{ + context::PrivateContext, + macros::notes::custom_note, + note::note_interface::NoteHash, + protocol::{ + address::AztecAddress, constants::DOM_SEP__NOTE_HASH, hash::poseidon2_hash_with_separator, + traits::Packable, + }, +}; + +#[derive(Eq, Packable)] +#[custom_note] +pub struct TestNote { + pub value: Field, +} + +pub global TEST_NOTE_NULLIFIER: Field = 2; + +impl NoteHash for TestNote { + fn compute_note_hash( + self, + _owner: AztecAddress, + storage_slot: Field, + randomness: Field, + ) -> Field { + let inputs = self.pack().concat([storage_slot, randomness]); + poseidon2_hash_with_separator(inputs, DOM_SEP__NOTE_HASH) + } + + fn compute_nullifier( + _self: Self, + _context: &mut PrivateContext, + _owner: AztecAddress, + _note_hash_for_nullification: Field, + ) -> Field { + // Not used in any meaningful way + 0 + } + + unconstrained fn compute_nullifier_unconstrained( + _self: Self, + _owner: AztecAddress, + _note_hash_for_nullification: Field, + ) -> Option { + // Returns a hardcoded value so we can verify that `_compute_note_hash_and_nullifier` propagates it correctly. + Option::some(TEST_NOTE_NULLIFIER) + } +} diff --git a/playground/yarn.lock b/playground/yarn.lock index b35c390cd3df..3cead50ac4e4 100644 --- a/playground/yarn.lock +++ b/playground/yarn.lock @@ -5873,8 +5873,8 @@ __metadata: linkType: hard "tar@npm:^7.4.3": - version: 7.4.3 - resolution: "tar@npm:7.4.3" + version: 7.5.11 + resolution: "tar@npm:7.5.11" dependencies: "@isaacs/fs-minipass": "npm:^4.0.0" chownr: "npm:^3.0.0" @@ -5882,7 +5882,7 @@ __metadata: minizlib: "npm:^3.0.1" mkdirp: "npm:^3.0.1" yallist: "npm:^5.0.0" - checksum: 10c0/d4679609bb2a9b48eeaf84632b6d844128d2412b95b6de07d53d8ee8baf4ca0857c9331dfa510390a0727b550fd543d4d1a10995ad86cdf078423fbb8d99831d + checksum: 10c0/b6bb420550ef50ef23356018155e956cd83282c97b6128d8d5cfe5740c57582d806a244b2ef0bf686a74ce526babe8b8b9061527623e935e850008d86d838929 languageName: node linkType: hard diff --git a/yarn-project/end-to-end/src/e2e_epochs/epochs_multiple.test.ts b/yarn-project/end-to-end/src/e2e_epochs/epochs_multiple.test.ts index 6703786b0f88..ac53842785d2 100644 --- a/yarn-project/end-to-end/src/e2e_epochs/epochs_multiple.test.ts +++ b/yarn-project/end-to-end/src/e2e_epochs/epochs_multiple.test.ts @@ -4,14 +4,12 @@ import { BlockNumber } from '@aztec/foundation/branded-types'; import { jest } from '@jest/globals'; -import type { EndToEndContext } from '../fixtures/utils.js'; -import { EpochsTestContext, WORLD_STATE_CHECKPOINT_HISTORY } from './epochs_test.js'; +import { EpochsTestContext } from './epochs_test.js'; jest.setTimeout(1000 * 60 * 15); // Assumes one block per checkpoint describe('e2e_epochs/epochs_multiple', () => { - let context: EndToEndContext; let rollup: RollupContract; let logger: Logger; @@ -19,7 +17,7 @@ describe('e2e_epochs/epochs_multiple', () => { beforeEach(async () => { test = await EpochsTestContext.setup(); - ({ context, rollup, logger } = test); + ({ rollup, logger } = test); }); afterEach(async () => { @@ -46,20 +44,6 @@ describe('e2e_epochs/epochs_multiple', () => { // Verify the state syncs. Assumes one block per checkpoint. const epochEndBlockNumber = BlockNumber.fromCheckpointNumber(epochEndCheckpointNumber); await test.waitForNodeToSync(epochEndBlockNumber, 'proven'); - await test.verifyHistoricBlock(epochEndBlockNumber, true); - - // Check that finalized blocks are purged from world state - // Right now finalization means a checkpoint is two L2 epochs deep. If this rule changes then this test needs to be updated. - // This test is setup as 1 block per checkpoint - const provenBlockNumber = epochEndBlockNumber; - const finalizedBlockNumber = Math.max(provenBlockNumber - context.config.aztecEpochDuration * 2, 0); - const expectedOldestHistoricBlock = Math.max(finalizedBlockNumber - WORLD_STATE_CHECKPOINT_HISTORY + 1, 1); - const expectedBlockRemoved = expectedOldestHistoricBlock - 1; - await test.waitForNodeToSync(BlockNumber(expectedOldestHistoricBlock), 'historic'); - await test.verifyHistoricBlock(BlockNumber(expectedOldestHistoricBlock), true); - if (expectedBlockRemoved > 0) { - await test.verifyHistoricBlock(BlockNumber(expectedBlockRemoved), false); - } } logger.info('Test Succeeded'); }); diff --git a/yarn-project/native/src/native_module.ts b/yarn-project/native/src/native_module.ts index 6966e33193d7..0319bf8d894b 100644 --- a/yarn-project/native/src/native_module.ts +++ b/yarn-project/native/src/native_module.ts @@ -128,33 +128,23 @@ export function cancelSimulation(token: CancellationToken): void { } /** - * Maximum number of concurrent AVM simulations. Each simulation spawns a dedicated OS thread, - * so this controls resource usage. Defaults to 4. Set to 0 for unlimited. + * Concurrency limiting for C++ AVM simulation to prevent libuv thread pool exhaustion. + * + * The C++ simulator uses NAPI BlockingCall to callback to TypeScript for contract data. + * This blocks the libuv thread while waiting for the callback to complete. If all libuv + * threads are blocked waiting for callbacks, no threads remain to service those callbacks, + * causing deadlock. + * + * We limit concurrent simulations to UV_THREADPOOL_SIZE / 2 to ensure threads remain + * available for callback processing. */ -export const AVM_MAX_CONCURRENT_SIMULATIONS = parseInt(process.env.AVM_MAX_CONCURRENT_SIMULATIONS ?? '4', 10); -const avmSimulationSemaphore = - AVM_MAX_CONCURRENT_SIMULATIONS > 0 ? new Semaphore(AVM_MAX_CONCURRENT_SIMULATIONS) : null; - -async function withAvmConcurrencyLimit(fn: () => Promise): Promise { - if (!avmSimulationSemaphore) { - return fn(); - } - await avmSimulationSemaphore.acquire(); - try { - return await fn(); - } finally { - avmSimulationSemaphore.release(); - } -} +const UV_THREADPOOL_SIZE = parseInt(process.env.UV_THREADPOOL_SIZE ?? '4', 10); +export const AVM_MAX_CONCURRENT_SIMULATIONS = Math.max(1, Math.floor(UV_THREADPOOL_SIZE / 2)); +const avmSimulationSemaphore = new Semaphore(AVM_MAX_CONCURRENT_SIMULATIONS); /** * AVM simulation function that takes serialized inputs and a contract provider. * The contract provider enables C++ to callback to TypeScript for contract data during simulation. - * - * Simulations run on dedicated std::threads (not the libuv thread pool), so there is no risk - * of libuv thread pool exhaustion or deadlock from C++ BlockingCall callbacks. - * Concurrency is limited by AVM_MAX_CONCURRENT_SIMULATIONS (default 4, 0 = unlimited). - * * @param inputs - Msgpack-serialized AvmFastSimulationInputs buffer * @param contractProvider - Object with callbacks for fetching contract instances and classes * @param worldStateHandle - Native handle to WorldState instance @@ -163,7 +153,7 @@ async function withAvmConcurrencyLimit(fn: () => Promise): Promise { * @param cancellationToken - Optional token to enable cancellation support * @returns Promise resolving to msgpack-serialized AvmCircuitPublicInputs buffer */ -export function avmSimulate( +export async function avmSimulate( inputs: Buffer, contractProvider: ContractProvider, worldStateHandle: any, @@ -171,30 +161,35 @@ export function avmSimulate( logger?: Logger, cancellationToken?: CancellationToken, ): Promise { - return withAvmConcurrencyLimit(() => - nativeAvmSimulate( + await avmSimulationSemaphore.acquire(); + + try { + return await nativeAvmSimulate( inputs, contractProvider, worldStateHandle, LogLevels.indexOf(logLevel), logger ? (level: LogLevel, msg: string) => logger[level](msg) : null, cancellationToken, - ), - ); + ); + } finally { + avmSimulationSemaphore.release(); + } } /** * AVM simulation function that uses pre-collected hints from TypeScript simulation. * All contract data and merkle tree hints are included in the AvmCircuitInputs, so no runtime * callbacks to TS or WS pointer are needed. - * - * Simulations run on dedicated std::threads (not the libuv thread pool). - * Concurrency is limited by AVM_MAX_CONCURRENT_SIMULATIONS (default 4, 0 = unlimited). - * * @param inputs - Msgpack-serialized AvmCircuitInputs (AvmProvingInputs in C++) buffer * @param logLevel - Log level to control C++ verbosity * @returns Promise resolving to msgpack-serialized simulation results buffer */ -export function avmSimulateWithHintedDbs(inputs: Buffer, logLevel: LogLevel = 'info'): Promise { - return withAvmConcurrencyLimit(() => nativeAvmSimulateWithHintedDbs(inputs, LogLevels.indexOf(logLevel))); +export async function avmSimulateWithHintedDbs(inputs: Buffer, logLevel: LogLevel = 'info'): Promise { + await avmSimulationSemaphore.acquire(); + try { + return await nativeAvmSimulateWithHintedDbs(inputs, LogLevels.indexOf(logLevel)); + } finally { + avmSimulationSemaphore.release(); + } } diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index a6bf73c00280..c8d238e6d2a4 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -43,6 +43,9 @@ export interface P2PConfig /** Maximum transactions per block for validation. Overrides maxTxsPerBlock for gossip validation when set. */ validateMaxTxsPerBlock?: number; + /** Maximum transactions per checkpoint for validation. Used as fallback for maxTxsPerBlock when that is not set. */ + validateMaxTxsPerCheckpoint?: number; + /** Maximum L2 gas per block for validation. When set, txs exceeding this limit are rejected. */ validateMaxL2BlockGas?: number; @@ -217,6 +220,12 @@ export const p2pConfigMappings: ConfigMappingsType = { 'Maximum transactions per block for validation. Overrides maxTxsPerBlock for gossip validation when set.', parseEnv: (val: string) => (val ? parseInt(val, 10) : undefined), }, + validateMaxTxsPerCheckpoint: { + env: 'VALIDATOR_MAX_TX_PER_CHECKPOINT', + description: + 'Maximum transactions per checkpoint for validation. Used as fallback for maxTxsPerBlock when that is not set.', + parseEnv: (val: string) => (val ? parseInt(val, 10) : undefined), + }, validateMaxL2BlockGas: { env: 'VALIDATOR_MAX_L2_BLOCK_GAS', description: 'Maximum L2 gas per block for validation. When set, txs exceeding this limit are rejected.', diff --git a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts index 061263fb20c7..4f8745d5cb86 100644 --- a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts +++ b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts @@ -226,7 +226,7 @@ export class LibP2PService extends WithTracer implements P2PService { const proposalValidatorOpts = { txsPermitted: !config.disableTransactions, - maxTxsPerBlock: config.validateMaxTxsPerBlock, + maxTxsPerBlock: config.validateMaxTxsPerBlock ?? config.validateMaxTxsPerCheckpoint, }; this.blockProposalValidator = new BlockProposalValidator(epochCache, proposalValidatorOpts); this.checkpointProposalValidator = new CheckpointProposalValidator(epochCache, proposalValidatorOpts); diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts index 3543c78fc84f..f4f685812c0a 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts @@ -22,9 +22,8 @@ import { Checkpoint, type CheckpointData, L1PublishedData } from '@aztec/stdlib/ import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers'; import { GasFees } from '@aztec/stdlib/gas'; import { - type BuildBlockInCheckpointResult, + InsufficientValidTxsError, type MerkleTreeWriteOperations, - NoValidTxsError, type ResolvedSequencerConfig, type WorldStateSynchronizer, } from '@aztec/stdlib/interfaces/server'; @@ -774,7 +773,7 @@ describe('CheckpointProposalJob', () => { const checkpointBuilder = mock(); const failedTxs: FailedTx[] = txs.slice(1).map(tx => ({ tx, error: new Error('Invalid tx') })); - checkpointBuilder.buildBlock.mockResolvedValue({ failedTxs, numTxs: 1 } as BuildBlockInCheckpointResult); + checkpointBuilder.buildBlock.mockRejectedValue(new InsufficientValidTxsError(1, 2, failedTxs)); const checkpoint = await job.buildSingleBlock(checkpointBuilder, { blockNumber: newBlockNumber, @@ -795,7 +794,7 @@ describe('CheckpointProposalJob', () => { const checkpointBuilder = mock(); const failedTxs: FailedTx[] = txs.slice(1).map(tx => ({ tx, error: new Error('Invalid tx') })); - checkpointBuilder.buildBlock.mockRejectedValue(new NoValidTxsError(failedTxs)); + checkpointBuilder.buildBlock.mockRejectedValue(new InsufficientValidTxsError(0, 3, failedTxs)); const checkpoint = await job.buildSingleBlock(checkpointBuilder, { blockNumber: newBlockNumber, diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index 4d38f965723e..eb52e6ae3463 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -34,7 +34,7 @@ import { type Checkpoint, validateCheckpoint } from '@aztec/stdlib/checkpoint'; import { getSlotStartBuildTimestamp } from '@aztec/stdlib/epoch-helpers'; import { Gas } from '@aztec/stdlib/gas'; import { - NoValidTxsError, + InsufficientValidTxsError, type PublicProcessorLimits, type ResolvedSequencerConfig, type WorldStateSynchronizer, @@ -568,7 +568,9 @@ export class CheckpointProposalJob implements Traceable { // Per-block limits derived at startup by computeBlockLimits(), further capped // by remaining checkpoint-level budgets inside CheckpointBuilder before each block is built. - const blockBuilderOptions: PublicProcessorLimits = { + // minValidTxs is passed into the builder so it can reject the block *before* updating state. + const minValidTxs = forceCreate ? 0 : (this.config.minValidTxsPerBlock ?? minTxs); + const blockBuilderOptions: PublicProcessorLimits & { minValidTxs?: number } = { maxTransactions: this.config.maxTxsPerBlock, maxBlockGas: this.config.maxL2BlockGas !== undefined || this.config.maxDABlockGas !== undefined @@ -576,9 +578,12 @@ export class CheckpointProposalJob implements Traceable { : undefined, deadline: buildDeadline, isBuildingProposal: true, + minValidTxs, }; - // Actually build the block by executing txs + // Actually build the block by executing txs. The builder throws InsufficientValidTxsError + // if the number of successfully processed txs is below minValidTxs, ensuring state is not + // updated for blocks that will be discarded. const buildResult = await this.buildSingleBlockWithCheckpointBuilder( checkpointBuilder, pendingTxs, @@ -590,14 +595,16 @@ export class CheckpointProposalJob implements Traceable { // If any txs failed during execution, drop them from the mempool so we don't pick them up again await this.dropFailedTxsFromP2P(buildResult.failedTxs); - // Check if we have created a block with enough txs. If there were invalid txs in the pool, or if execution took - // too long, then we may not get to minTxsPerBlock after executing public functions. - const minValidTxs = this.config.minValidTxsPerBlock ?? minTxs; - const numTxs = buildResult.status === 'no-valid-txs' ? 0 : buildResult.numTxs; - if (buildResult.status === 'no-valid-txs' || (!forceCreate && numTxs < minValidTxs)) { + if (buildResult.status === 'insufficient-valid-txs') { this.log.warn( `Block ${blockNumber} at index ${indexWithinCheckpoint} on slot ${this.slot} has too few valid txs to be proposed`, - { slot: this.slot, blockNumber, numTxs, indexWithinCheckpoint, minValidTxs, buildResult: buildResult.status }, + { + slot: this.slot, + blockNumber, + numTxs: buildResult.processedCount, + indexWithinCheckpoint, + minValidTxs, + }, ); this.eventEmitter.emit('block-build-failed', { reason: `Insufficient valid txs`, slot: this.slot }); this.metrics.recordBlockProposalFailed('insufficient_valid_txs'); @@ -605,7 +612,7 @@ export class CheckpointProposalJob implements Traceable { } // Block creation succeeded, emit stats and metrics - const { block, publicProcessorDuration, usedTxs, blockBuildDuration } = buildResult; + const { block, publicProcessorDuration, usedTxs, blockBuildDuration, numTxs } = buildResult; const blockStats = { eventName: 'l2-block-built', @@ -636,13 +643,13 @@ export class CheckpointProposalJob implements Traceable { } } - /** Uses the checkpoint builder to build a block, catching specific txs */ + /** Uses the checkpoint builder to build a block, catching InsufficientValidTxsError. */ private async buildSingleBlockWithCheckpointBuilder( checkpointBuilder: CheckpointBuilder, pendingTxs: AsyncIterable, blockNumber: BlockNumber, blockTimestamp: bigint, - blockBuilderOptions: PublicProcessorLimits, + blockBuilderOptions: PublicProcessorLimits & { minValidTxs?: number }, ) { try { const workTimer = new Timer(); @@ -650,8 +657,12 @@ export class CheckpointProposalJob implements Traceable { const blockBuildDuration = workTimer.ms(); return { ...result, blockBuildDuration, status: 'success' as const }; } catch (err: unknown) { - if (isErrorClass(err, NoValidTxsError)) { - return { failedTxs: err.failedTxs, status: 'no-valid-txs' as const }; + if (isErrorClass(err, InsufficientValidTxsError)) { + return { + failedTxs: err.failedTxs, + processedCount: err.processedCount, + status: 'insufficient-valid-txs' as const, + }; } throw err; } diff --git a/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts b/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts index 42d691191ef8..3c737f0968ef 100644 --- a/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts +++ b/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts @@ -32,7 +32,7 @@ export class MockCheckpointBuilder implements ICheckpointBlockBuilder { public buildBlockCalls: Array<{ blockNumber: BlockNumber; timestamp: bigint; - opts: PublicProcessorLimits; + opts: PublicProcessorLimits & { minValidTxs?: number }; }> = []; /** Track all consumed transaction hashes across buildBlock calls */ public consumedTxHashes: Set = new Set(); @@ -74,7 +74,7 @@ export class MockCheckpointBuilder implements ICheckpointBlockBuilder { pendingTxs: Iterable | AsyncIterable, blockNumber: BlockNumber, timestamp: bigint, - opts: PublicProcessorLimits, + opts: PublicProcessorLimits & { minValidTxs?: number }, ): Promise { this.buildBlockCalls.push({ blockNumber, timestamp, opts }); diff --git a/yarn-project/simulator/src/public/hinting_db_sources.ts b/yarn-project/simulator/src/public/hinting_db_sources.ts index 79044c631e64..85f8ab422ccf 100644 --- a/yarn-project/simulator/src/public/hinting_db_sources.ts +++ b/yarn-project/simulator/src/public/hinting_db_sources.ts @@ -410,12 +410,12 @@ export class HintingMerkleWriteOperations implements MerkleTreeWriteOperations { } } - public async createCheckpoint(): Promise { + public async createCheckpoint(): Promise { const actionCounter = this.checkpointActionCounter++; const oldCheckpointId = this.getCurrentCheckpointId(); const treesStateHash = await this.getTreesStateHash(); - await this.db.createCheckpoint(); + const depth = await this.db.createCheckpoint(); this.checkpointStack.push(this.nextCheckpointId++); const newCheckpointId = this.getCurrentCheckpointId(); @@ -424,14 +424,16 @@ export class HintingMerkleWriteOperations implements MerkleTreeWriteOperations { HintingMerkleWriteOperations.log.trace( `[createCheckpoint:${actionCounter}] Checkpoint evolved ${oldCheckpointId} -> ${newCheckpointId} at trees state ${treesStateHash}.`, ); + + return depth; } - public commitAllCheckpoints(): Promise { - throw new Error('commitAllCheckpoints is not supported in HintingMerkleWriteOperations.'); + public commitAllCheckpointsTo(_depth: number): Promise { + throw new Error('commitAllCheckpointsTo is not supported in HintingMerkleWriteOperations.'); } - public revertAllCheckpoints(): Promise { - throw new Error('revertAllCheckpoints is not supported in HintingMerkleWriteOperations.'); + public revertAllCheckpointsTo(_depth: number): Promise { + throw new Error('revertAllCheckpointsTo is not supported in HintingMerkleWriteOperations.'); } public async commitCheckpoint(): Promise { diff --git a/yarn-project/simulator/src/public/public_processor/apps_tests/timeout_race.test.ts b/yarn-project/simulator/src/public/public_processor/apps_tests/timeout_race.test.ts index 3d06b2323916..2d16e26e602f 100644 --- a/yarn-project/simulator/src/public/public_processor/apps_tests/timeout_race.test.ts +++ b/yarn-project/simulator/src/public/public_processor/apps_tests/timeout_race.test.ts @@ -20,7 +20,7 @@ import { GasFees } from '@aztec/stdlib/gas'; import { MerkleTreeId, merkleTreeIds } from '@aztec/stdlib/trees'; import { GlobalVariables } from '@aztec/stdlib/tx'; import { getTelemetryClient } from '@aztec/telemetry-client'; -import { NativeWorldStateService } from '@aztec/world-state'; +import { ForkCheckpoint, NativeWorldStateService } from '@aztec/world-state'; import { jest } from '@jest/globals'; @@ -115,7 +115,7 @@ describe('PublicProcessor C++ Timeout Race Condition', () => { } // Create checkpoint BEFORE simulation (like PublicProcessor does) - await merkleTrees.createCheckpoint(); + const forkCheckpoint = await ForkCheckpoint.new(merkleTrees); // Create transaction that calls the spammer contract const tx = await tester.createTx(admin, [], [{ address: contractAddress, args: callArgs }]); @@ -136,11 +136,8 @@ describe('PublicProcessor C++ Timeout Race Condition', () => { } // BUG - No cancel, C++ continues running during reverts below - // Revert checkpoint - await merkleTrees.revertCheckpoint(); - - // Clean up - await merkleTrees.revertAllCheckpoints(); + // Clean up - revert all changes + await forkCheckpoint.revertToCheckpoint(); // Wait for simulation promise for cleanup await Promise.race([simulationPromise.catch(() => {}), sleep(100)]); diff --git a/yarn-project/simulator/src/public/public_processor/guarded_merkle_tree.ts b/yarn-project/simulator/src/public/public_processor/guarded_merkle_tree.ts index bcbd818a03f0..71133c4a2ebf 100644 --- a/yarn-project/simulator/src/public/public_processor/guarded_merkle_tree.ts +++ b/yarn-project/simulator/src/public/public_processor/guarded_merkle_tree.ts @@ -134,7 +134,7 @@ export class GuardedMerkleTreeOperations implements MerkleTreeWriteOperations { ): Promise<(BlockNumber | undefined)[]> { return this.guardAndPush(() => this.target.getBlockNumbersForLeafIndices(treeId, leafIndices)); } - createCheckpoint(): Promise { + createCheckpoint(): Promise { return this.guardAndPush(() => this.target.createCheckpoint()); } commitCheckpoint(): Promise { @@ -143,11 +143,11 @@ export class GuardedMerkleTreeOperations implements MerkleTreeWriteOperations { revertCheckpoint(): Promise { return this.guardAndPush(() => this.target.revertCheckpoint()); } - commitAllCheckpoints(): Promise { - return this.guardAndPush(() => this.target.commitAllCheckpoints()); + commitAllCheckpointsTo(depth: number): Promise { + return this.guardAndPush(() => this.target.commitAllCheckpointsTo(depth)); } - revertAllCheckpoints(): Promise { - return this.guardAndPush(() => this.target.revertAllCheckpoints()); + revertAllCheckpointsTo(depth: number): Promise { + return this.guardAndPush(() => this.target.revertAllCheckpointsTo(depth)); } findSiblingPaths( treeId: ID, diff --git a/yarn-project/simulator/src/public/public_processor/public_processor.test.ts b/yarn-project/simulator/src/public/public_processor/public_processor.test.ts index 907ee1f907c6..23a019bb6080 100644 --- a/yarn-project/simulator/src/public/public_processor/public_processor.test.ts +++ b/yarn-project/simulator/src/public/public_processor/public_processor.test.ts @@ -91,6 +91,7 @@ describe('public_processor', () => { new PublicDataTreeLeafPreimage(new PublicDataTreeLeaf(Fr.ZERO, Fr.ZERO), /*nextKey=*/ Fr.ZERO, /*nextIndex=*/ 0n), ); merkleTree.getStateReference.mockResolvedValue(stateReference); + merkleTree.createCheckpoint.mockResolvedValue(1); publicTxSimulator.simulate.mockImplementation(() => { return Promise.resolve(mockedEnqueuedCallsResult); @@ -158,7 +159,7 @@ describe('public_processor', () => { expect(failed[0].error).toEqual(new Error(`Failed`)); expect(merkleTree.commitCheckpoint).toHaveBeenCalledTimes(0); - expect(merkleTree.revertCheckpoint).toHaveBeenCalledTimes(1); + expect(merkleTree.revertAllCheckpointsTo).toHaveBeenCalledWith(0); }); it('if a tx errors with assertion failure, public processor returns failed tx with its assertion message', async function () { @@ -173,7 +174,7 @@ describe('public_processor', () => { expect(failed[0].error.message).toMatch(/Forced assertion failure/); expect(merkleTree.commitCheckpoint).toHaveBeenCalledTimes(0); - expect(merkleTree.revertCheckpoint).toHaveBeenCalledTimes(1); + expect(merkleTree.revertAllCheckpointsTo).toHaveBeenCalledWith(0); }); it('does not attempt to overfill a block', async function () { @@ -314,11 +315,45 @@ describe('public_processor', () => { expect(failed[0].error.message).toMatch(/Not enough balance/i); expect(merkleTree.commitCheckpoint).toHaveBeenCalledTimes(0); - expect(merkleTree.revertCheckpoint).toHaveBeenCalledTimes(1); + expect(merkleTree.revertAllCheckpointsTo).toHaveBeenCalledWith(0); expect(merkleTree.sequentialInsert).toHaveBeenCalledTimes(0); }); }); + describe('checkpoint depth', () => { + it('calls revertAllCheckpointsTo with depth on tx failure', async function () { + merkleTree.createCheckpoint.mockResolvedValue(2); + publicTxSimulator.simulate.mockRejectedValue(new Error('Boom')); + + const tx = await mockTxWithPublicCalls(); + const [processed, failed] = await processor.process([tx]); + + expect(processed).toEqual([]); + expect(failed).toHaveLength(1); + expect(merkleTree.revertAllCheckpointsTo).toHaveBeenCalledWith(1); + expect(merkleTree.commitCheckpoint).not.toHaveBeenCalled(); + }); + + it('createCheckpoint is called for each tx', async function () { + const txs = await timesParallel(3, () => mockPrivateOnlyTx()); + + await processor.process(txs); + + expect(merkleTree.createCheckpoint).toHaveBeenCalledTimes(3); + }); + + it('commits checkpoint on successful tx', async function () { + const tx = await mockTxWithPublicCalls(); + + const [processed, failed] = await processor.process([tx]); + + expect(processed).toHaveLength(1); + expect(failed).toEqual([]); + expect(merkleTree.commitCheckpoint).toHaveBeenCalledTimes(1); + expect(merkleTree.revertAllCheckpointsTo).not.toHaveBeenCalled(); + }); + }); + // on uncaught error, public processor clears the tx-level cache entirely it('clears the tx-level cache entirely on uncaught error (like SETUP failure)', async function () { const tx = await mockTxWithPublicCalls(); diff --git a/yarn-project/simulator/src/public/public_processor/public_processor.ts b/yarn-project/simulator/src/public/public_processor/public_processor.ts index 45a3d9e6906e..20ce6fbaa3e4 100644 --- a/yarn-project/simulator/src/public/public_processor/public_processor.ts +++ b/yarn-project/simulator/src/public/public_processor/public_processor.ts @@ -325,14 +325,10 @@ export class PublicProcessor implements Traceable { // 1. At least one outstanding checkpoint that has not been committed (the one created before we processed the tx). // 2. Possible state updates on that checkpoint or any others created during execution. - // First we revert a checkpoint as managed by the ForkCheckpoint. This will revert whatever is the current checkpoint - // which may not be the one originally created by this object. But that is ok, we do this to fulfil the ForkCheckpoint - // lifecycle expectations and ensure it doesn't attempt to commit later on. - await checkpoint.revert(); - - // Now we want to revert any/all remaining checkpoints, destroying any outstanding state updates. - // This needs to be done directly on the underlying fork as the guarded fork has been stopped. - await this.guardedMerkleTree.getUnderlyingFork().revertAllCheckpoints(); + // Revert all checkpoints at or above this checkpoint's depth (inclusive), destroying any outstanding state + // updates from this tx and any nested checkpoints created during execution. This preserves any checkpoints + // created by callers below our depth. + await checkpoint.revertToCheckpoint(); // Revert any contracts added to the DB for the tx. this.contractsDB.revertCheckpoint(); @@ -344,9 +340,9 @@ export class PublicProcessor implements Traceable { break; } - // Roll back state to start of TX before proceeding to next TX - await checkpoint.revert(); - await this.guardedMerkleTree.getUnderlyingFork().revertAllCheckpoints(); + // Roll back state to start of TX before proceeding to next TX. + // Reverts all checkpoints at or above this checkpoint's depth, preserving any caller checkpoints below. + await checkpoint.revertToCheckpoint(); this.contractsDB.revertCheckpoint(); const errorMessage = err instanceof Error || err instanceof AssertionError ? err.message : 'Unknown error'; this.log.warn(`Failed to process tx ${txHash.toString()}: ${errorMessage} ${err?.stack}`); diff --git a/yarn-project/stdlib/src/interfaces/block-builder.ts b/yarn-project/stdlib/src/interfaces/block-builder.ts index 7b79c7134760..5d6460751af7 100644 --- a/yarn-project/stdlib/src/interfaces/block-builder.ts +++ b/yarn-project/stdlib/src/interfaces/block-builder.ts @@ -81,11 +81,15 @@ export const FullNodeBlockBuilderConfigKeys: (keyof FullNodeBlockBuilderConfig)[ 'rollupManaLimit', ] as const; -/** Thrown when no valid transactions are available to include in a block after processing, and this is not the first block in a checkpoint. */ -export class NoValidTxsError extends Error { - constructor(public readonly failedTxs: FailedTx[]) { - super('No valid transactions to include in block'); - this.name = 'NoValidTxsError'; +/** Thrown when the number of successfully processed transactions is below the required minimum. */ +export class InsufficientValidTxsError extends Error { + constructor( + public readonly processedCount: number, + public readonly minRequired: number, + public readonly failedTxs: FailedTx[], + ) { + super(`Insufficient valid txs: got ${processedCount} but need ${minRequired}`); + this.name = 'InsufficientValidTxsError'; } } @@ -100,11 +104,12 @@ export type BuildBlockInCheckpointResult = { /** Interface for building blocks within a checkpoint context. */ export interface ICheckpointBlockBuilder { + /** Builds a single block within this checkpoint. Throws InsufficientValidTxsError if fewer than minValidTxs succeed. */ buildBlock( pendingTxs: Iterable | AsyncIterable, blockNumber: BlockNumber, timestamp: bigint, - opts: PublicProcessorLimits, + opts: PublicProcessorLimits & { minValidTxs?: number }, ): Promise; } diff --git a/yarn-project/stdlib/src/interfaces/merkle_tree_operations.ts b/yarn-project/stdlib/src/interfaces/merkle_tree_operations.ts index 63ee8e82f9b1..29625e9d4c43 100644 --- a/yarn-project/stdlib/src/interfaces/merkle_tree_operations.ts +++ b/yarn-project/stdlib/src/interfaces/merkle_tree_operations.ts @@ -225,30 +225,20 @@ export interface MerkleTreeReadOperations { } export interface MerkleTreeCheckpointOperations { - /** - * Checkpoints the current fork state - */ - createCheckpoint(): Promise; + /** Checkpoints the current fork state. Returns the depth of the new checkpoint. */ + createCheckpoint(): Promise; - /** - * Commits the current checkpoint - */ + /** Commits the current checkpoint. */ commitCheckpoint(): Promise; - /** - * Reverts the current checkpoint - */ + /** Reverts the current checkpoint. */ revertCheckpoint(): Promise; - /** - * Commits all checkpoints - */ - commitAllCheckpoints(): Promise; + /** Commits all checkpoints above the given depth, leaving checkpoint depth at the given value. */ + commitAllCheckpointsTo(depth: number): Promise; - /** - * Reverts all checkpoints - */ - revertAllCheckpoints(): Promise; + /** Reverts all checkpoints above the given depth, leaving checkpoint depth at the given value. */ + revertAllCheckpointsTo(depth: number): Promise; } export interface MerkleTreeWriteOperations diff --git a/yarn-project/validator-client/src/checkpoint_builder.test.ts b/yarn-project/validator-client/src/checkpoint_builder.test.ts index 0d9cf8ae6959..cbc6fe3b43f5 100644 --- a/yarn-project/validator-client/src/checkpoint_builder.test.ts +++ b/yarn-project/validator-client/src/checkpoint_builder.test.ts @@ -17,11 +17,12 @@ import type { ContractDataSource } from '@aztec/stdlib/contract'; import { Gas, GasFees } from '@aztec/stdlib/gas'; import { type FullNodeBlockBuilderConfig, + InsufficientValidTxsError, type MerkleTreeWriteOperations, - NoValidTxsError, type PublicProcessorLimits, type PublicProcessorValidator, } from '@aztec/stdlib/interfaces/server'; +import { TxHash } from '@aztec/stdlib/tx'; import type { CheckpointGlobalVariables, GlobalVariables, ProcessedTx, Tx } from '@aztec/stdlib/tx'; import type { TelemetryClient } from '@aztec/telemetry-client'; @@ -138,9 +139,7 @@ describe('CheckpointBuilder', () => { expect(lightweightCheckpointBuilder.addBlock).toHaveBeenCalled(); }); - it('allows building an empty first block in a checkpoint', async () => { - lightweightCheckpointBuilder.getBlockCount.mockReturnValue(0); - + it('allows building an empty block when minValidTxs is 0', async () => { const expectedBlock = await L2Block.random(blockNumber, { txsPerBlock: 0 }); lightweightCheckpointBuilder.addBlock.mockResolvedValue(expectedBlock); @@ -153,16 +152,14 @@ describe('CheckpointBuilder', () => { [], // debugLogs ]); - const result = await checkpointBuilder.buildBlock([], blockNumber, 1000n); + const result = await checkpointBuilder.buildBlock([], blockNumber, 1000n, { minValidTxs: 0 }); expect(result.block).toBe(expectedBlock); expect(result.numTxs).toBe(0); expect(lightweightCheckpointBuilder.addBlock).toHaveBeenCalled(); }); - it('throws NoValidTxsError when no valid transactions and not first block in checkpoint', async () => { - lightweightCheckpointBuilder.getBlockCount.mockReturnValue(1); - + it('throws InsufficientValidTxsError when fewer txs than minValidTxs', async () => { const failedTx = { tx: { txHash: Fr.random() } as unknown as Tx, error: new Error('tx failed') }; processor.process.mockResolvedValue([ [], // processedTxs - empty @@ -172,10 +169,46 @@ describe('CheckpointBuilder', () => { [], // debugLogs ]); - await expect(checkpointBuilder.buildBlock([], blockNumber, 1000n)).rejects.toThrow(NoValidTxsError); + await expect(checkpointBuilder.buildBlock([], blockNumber, 1000n, { minValidTxs: 1 })).rejects.toThrow( + InsufficientValidTxsError, + ); expect(lightweightCheckpointBuilder.addBlock).not.toHaveBeenCalled(); }); + + it('does not update state when some txs succeed but below minValidTxs', async () => { + const processedTx = mock(); + processedTx.hash = TxHash.random(); + const failedTx = { tx: { txHash: Fr.random() } as unknown as Tx, error: new Error('tx failed') }; + processor.process.mockResolvedValue([ + [processedTx], // processedTxs - 1 succeeded + [failedTx], // failedTxs - 1 failed + [], // usedTxs + [], // returnValues + [], // debugLogs + ]); + + const err = await checkpointBuilder + .buildBlock([], blockNumber, 1000n, { minValidTxs: 2 }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(InsufficientValidTxsError); + expect((err as InsufficientValidTxsError).processedCount).toBe(1); + expect((err as InsufficientValidTxsError).minRequired).toBe(2); + expect(lightweightCheckpointBuilder.addBlock).not.toHaveBeenCalled(); + }); + + it('defaults to minValidTxs=0 when not specified, allowing empty blocks', async () => { + const expectedBlock = await L2Block.random(blockNumber, { txsPerBlock: 0 }); + lightweightCheckpointBuilder.addBlock.mockResolvedValue(expectedBlock); + + processor.process.mockResolvedValue([[], [], [], [], []]); + + const result = await checkpointBuilder.buildBlock([], blockNumber, 1000n); + + expect(result.numTxs).toBe(0); + expect(lightweightCheckpointBuilder.addBlock).toHaveBeenCalled(); + }); }); describe('capLimitsByCheckpointBudgets', () => { diff --git a/yarn-project/validator-client/src/checkpoint_builder.ts b/yarn-project/validator-client/src/checkpoint_builder.ts index a80b3d2697b1..b65a9127955f 100644 --- a/yarn-project/validator-client/src/checkpoint_builder.ts +++ b/yarn-project/validator-client/src/checkpoint_builder.ts @@ -25,8 +25,8 @@ import { FullNodeBlockBuilderConfigKeys, type ICheckpointBlockBuilder, type ICheckpointsBuilder, + InsufficientValidTxsError, type MerkleTreeWriteOperations, - NoValidTxsError, type PublicProcessorLimits, type WorldStateSynchronizer, } from '@aztec/stdlib/interfaces/server'; @@ -34,6 +34,7 @@ import { type DebugLogStore, NullDebugLogStore } from '@aztec/stdlib/logs'; import { MerkleTreeId } from '@aztec/stdlib/trees'; import { type CheckpointGlobalVariables, GlobalVariables, StateReference, Tx } from '@aztec/stdlib/tx'; import { type TelemetryClient, getTelemetryClient } from '@aztec/telemetry-client'; +import { ForkCheckpoint } from '@aztec/world-state'; // Re-export for backward compatibility export type { BuildBlockInCheckpointResult } from '@aztec/stdlib/interfaces/server'; @@ -73,7 +74,7 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder { pendingTxs: Iterable | AsyncIterable, blockNumber: BlockNumber, timestamp: bigint, - opts: PublicProcessorLimits & { expectedEndState?: StateReference } = {}, + opts: PublicProcessorLimits & { expectedEndState?: StateReference; minValidTxs?: number } = {}, ): Promise { const slot = this.checkpointBuilder.constants.slotNumber; @@ -103,34 +104,47 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder { ...this.capLimitsByCheckpointBudgets(opts), }; - const [publicProcessorDuration, [processedTxs, failedTxs, usedTxs]] = await elapsed(() => - processor.process(pendingTxs, cappedOpts, validator), - ); + // We execute all merkle tree operations on a world state fork checkpoint + // This enables us to discard all modifications in the event that we fail to successfully process sufficient transactions + const forkCheckpoint = await ForkCheckpoint.new(this.fork); - // Throw if we didn't collect a single valid tx and we're not allowed to build empty blocks - // (only the first block in a checkpoint can be empty) - if (processedTxs.length === 0 && this.checkpointBuilder.getBlockCount() > 0) { - throw new NoValidTxsError(failedTxs); + try { + const [publicProcessorDuration, [processedTxs, failedTxs, usedTxs]] = await elapsed(() => + processor.process(pendingTxs, cappedOpts, validator), + ); + // Throw before updating state if we don't have enough valid txs + const minValidTxs = opts.minValidTxs ?? 0; + if (processedTxs.length < minValidTxs) { + throw new InsufficientValidTxsError(processedTxs.length, minValidTxs, failedTxs); + } + + // Commit the fork checkpoint + await forkCheckpoint.commit(); + + // Add block to checkpoint + const block = await this.checkpointBuilder.addBlock(globalVariables, processedTxs, { + expectedEndState: opts.expectedEndState, + }); + + this.log.debug('Built block within checkpoint', { + header: block.header.toInspect(), + processedTxs: processedTxs.map(tx => tx.hash.toString()), + failedTxs: failedTxs.map(tx => tx.tx.txHash.toString()), + }); + + return { + block, + publicProcessorDuration, + numTxs: processedTxs.length, + failedTxs, + usedTxs, + }; + } catch (err) { + // If we reached the point of committing the checkpoint, this does nothing + // Otherwise it reverts any changes made to the fork for this failed block + await forkCheckpoint.revert(); + throw err; } - - // Add block to checkpoint - const block = await this.checkpointBuilder.addBlock(globalVariables, processedTxs, { - expectedEndState: opts.expectedEndState, - }); - - this.log.debug('Built block within checkpoint', { - header: block.header.toInspect(), - processedTxs: processedTxs.map(tx => tx.hash.toString()), - failedTxs: failedTxs.map(tx => tx.tx.txHash.toString()), - }); - - return { - block, - publicProcessorDuration, - numTxs: processedTxs.length, - failedTxs, - usedTxs, - }; } /** Completes the checkpoint and returns it. */ diff --git a/yarn-project/validator-client/src/factory.ts b/yarn-project/validator-client/src/factory.ts index b7645d48c485..6c706c5dc855 100644 --- a/yarn-project/validator-client/src/factory.ts +++ b/yarn-project/validator-client/src/factory.ts @@ -29,7 +29,7 @@ export function createBlockProposalHandler( const metrics = new ValidatorMetrics(deps.telemetry); const blockProposalValidator = new BlockProposalValidator(deps.epochCache, { txsPermitted: !config.disableTransactions, - maxTxsPerBlock: config.validateMaxTxsPerBlock, + maxTxsPerBlock: config.validateMaxTxsPerBlock ?? config.validateMaxTxsPerCheckpoint, }); return new BlockProposalHandler( deps.checkpointsBuilder, diff --git a/yarn-project/world-state/src/native/fork_checkpoint.test.ts b/yarn-project/world-state/src/native/fork_checkpoint.test.ts new file mode 100644 index 000000000000..787ccfab1221 --- /dev/null +++ b/yarn-project/world-state/src/native/fork_checkpoint.test.ts @@ -0,0 +1,71 @@ +import type { MerkleTreeCheckpointOperations } from '@aztec/stdlib/interfaces/server'; + +import { type MockProxy, mock } from 'jest-mock-extended'; + +import { ForkCheckpoint } from './fork_checkpoint.js'; + +describe('ForkCheckpoint', () => { + let fork: MockProxy; + + beforeEach(() => { + fork = mock(); + fork.createCheckpoint.mockResolvedValue(5); + fork.commitCheckpoint.mockResolvedValue(); + fork.revertCheckpoint.mockResolvedValue(); + }); + + it('stores depth from createCheckpoint', async () => { + const checkpoint = await ForkCheckpoint.new(fork); + expect(checkpoint.depth).toBe(5); + expect(fork.createCheckpoint).toHaveBeenCalledTimes(1); + }); + + it('commit calls commitCheckpoint on fork', async () => { + const checkpoint = await ForkCheckpoint.new(fork); + await checkpoint.commit(); + expect(fork.commitCheckpoint).toHaveBeenCalledTimes(1); + }); + + it('revert calls revertCheckpoint on fork', async () => { + const checkpoint = await ForkCheckpoint.new(fork); + await checkpoint.revert(); + expect(fork.revertCheckpoint).toHaveBeenCalledTimes(1); + }); + + it('revertToCheckpoint calls revertAllCheckpointsTo with depth', async () => { + fork.revertAllCheckpointsTo.mockResolvedValue(); + const checkpoint = await ForkCheckpoint.new(fork); + await checkpoint.revertToCheckpoint(); + expect(fork.revertAllCheckpointsTo).toHaveBeenCalledWith(4); + }); + + it('revertToCheckpoint prevents subsequent commit', async () => { + fork.revertAllCheckpointsTo.mockResolvedValue(); + const checkpoint = await ForkCheckpoint.new(fork); + await checkpoint.revertToCheckpoint(); + await checkpoint.commit(); + expect(fork.commitCheckpoint).not.toHaveBeenCalled(); + }); + + it('revertToCheckpoint is idempotent', async () => { + fork.revertAllCheckpointsTo.mockResolvedValue(); + const checkpoint = await ForkCheckpoint.new(fork); + await checkpoint.revertToCheckpoint(); + await checkpoint.revertToCheckpoint(); + expect(fork.revertAllCheckpointsTo).toHaveBeenCalledTimes(1); + }); + + it('commit is idempotent', async () => { + const checkpoint = await ForkCheckpoint.new(fork); + await checkpoint.commit(); + await checkpoint.commit(); + expect(fork.commitCheckpoint).toHaveBeenCalledTimes(1); + }); + + it('revert is idempotent', async () => { + const checkpoint = await ForkCheckpoint.new(fork); + await checkpoint.revert(); + await checkpoint.revert(); + expect(fork.revertCheckpoint).toHaveBeenCalledTimes(1); + }); +}); diff --git a/yarn-project/world-state/src/native/fork_checkpoint.ts b/yarn-project/world-state/src/native/fork_checkpoint.ts index c4172d689fc3..1672092ff0fe 100644 --- a/yarn-project/world-state/src/native/fork_checkpoint.ts +++ b/yarn-project/world-state/src/native/fork_checkpoint.ts @@ -3,11 +3,14 @@ import type { MerkleTreeCheckpointOperations } from '@aztec/stdlib/interfaces/se export class ForkCheckpoint { private completed = false; - private constructor(private readonly fork: MerkleTreeCheckpointOperations) {} + private constructor( + private readonly fork: MerkleTreeCheckpointOperations, + public readonly depth: number, + ) {} static async new(fork: MerkleTreeCheckpointOperations): Promise { - await fork.createCheckpoint(); - return new ForkCheckpoint(fork); + const depth = await fork.createCheckpoint(); + return new ForkCheckpoint(fork, depth); } async commit(): Promise { @@ -27,4 +30,17 @@ export class ForkCheckpoint { await this.fork.revertCheckpoint(); this.completed = true; } + + /** + * Reverts this checkpoint and any nested checkpoints created on top of it, + * leaving the checkpoint depth at the level it was before this checkpoint was created. + */ + async revertToCheckpoint(): Promise { + if (this.completed) { + return; + } + + await this.fork.revertAllCheckpointsTo(this.depth - 1); + this.completed = true; + } } diff --git a/yarn-project/world-state/src/native/merkle_trees_facade.ts b/yarn-project/world-state/src/native/merkle_trees_facade.ts index b7a107a8eb80..b8d4ca92b3e0 100644 --- a/yarn-project/world-state/src/native/merkle_trees_facade.ts +++ b/yarn-project/world-state/src/native/merkle_trees_facade.ts @@ -319,9 +319,10 @@ export class MerkleTreesForkFacade extends MerkleTreesFacade implements MerkleTr } } - public async createCheckpoint(): Promise { + public async createCheckpoint(): Promise { assert.notEqual(this.revision.forkId, 0, 'Fork ID must be set'); - await this.instance.call(WorldStateMessageType.CREATE_CHECKPOINT, { forkId: this.revision.forkId }); + const resp = await this.instance.call(WorldStateMessageType.CREATE_CHECKPOINT, { forkId: this.revision.forkId }); + return resp.depth; } public async commitCheckpoint(): Promise { @@ -334,14 +335,20 @@ export class MerkleTreesForkFacade extends MerkleTreesFacade implements MerkleTr await this.instance.call(WorldStateMessageType.REVERT_CHECKPOINT, { forkId: this.revision.forkId }); } - public async commitAllCheckpoints(): Promise { + public async commitAllCheckpointsTo(depth: number): Promise { assert.notEqual(this.revision.forkId, 0, 'Fork ID must be set'); - await this.instance.call(WorldStateMessageType.COMMIT_ALL_CHECKPOINTS, { forkId: this.revision.forkId }); + await this.instance.call(WorldStateMessageType.COMMIT_ALL_CHECKPOINTS, { + forkId: this.revision.forkId, + depth, + }); } - public async revertAllCheckpoints(): Promise { + public async revertAllCheckpointsTo(depth: number): Promise { assert.notEqual(this.revision.forkId, 0, 'Fork ID must be set'); - await this.instance.call(WorldStateMessageType.REVERT_ALL_CHECKPOINTS, { forkId: this.revision.forkId }); + await this.instance.call(WorldStateMessageType.REVERT_ALL_CHECKPOINTS, { + forkId: this.revision.forkId, + depth, + }); } } diff --git a/yarn-project/world-state/src/native/message.ts b/yarn-project/world-state/src/native/message.ts index 64f195918c32..edceed40e4b3 100644 --- a/yarn-project/world-state/src/native/message.ts +++ b/yarn-project/world-state/src/native/message.ts @@ -284,6 +284,16 @@ interface WithForkId { forkId: number; } +interface CreateCheckpointResponse { + depth: number; +} + +/** Request to commit/revert all checkpoints down to a target depth. The resulting depth after the operation equals the given depth. */ +interface CheckpointDepthRequest extends WithForkId { + /** The target depth after the operation. All checkpoints above this depth are committed/reverted. */ + depth: number; +} + interface WithWorldStateRevision { revision: WorldStateRevision; } @@ -487,8 +497,8 @@ export type WorldStateRequest = { [WorldStateMessageType.CREATE_CHECKPOINT]: WithForkId; [WorldStateMessageType.COMMIT_CHECKPOINT]: WithForkId; [WorldStateMessageType.REVERT_CHECKPOINT]: WithForkId; - [WorldStateMessageType.COMMIT_ALL_CHECKPOINTS]: WithForkId; - [WorldStateMessageType.REVERT_ALL_CHECKPOINTS]: WithForkId; + [WorldStateMessageType.COMMIT_ALL_CHECKPOINTS]: CheckpointDepthRequest; + [WorldStateMessageType.REVERT_ALL_CHECKPOINTS]: CheckpointDepthRequest; [WorldStateMessageType.COPY_STORES]: CopyStoresRequest; @@ -529,7 +539,7 @@ export type WorldStateResponse = { [WorldStateMessageType.GET_STATUS]: WorldStateStatusSummary; - [WorldStateMessageType.CREATE_CHECKPOINT]: void; + [WorldStateMessageType.CREATE_CHECKPOINT]: CreateCheckpointResponse; [WorldStateMessageType.COMMIT_CHECKPOINT]: void; [WorldStateMessageType.REVERT_CHECKPOINT]: void; [WorldStateMessageType.COMMIT_ALL_CHECKPOINTS]: void; diff --git a/yarn-project/world-state/src/native/native_world_state.test.ts b/yarn-project/world-state/src/native/native_world_state.test.ts index 9677c8698098..ea52aa4a20b3 100644 --- a/yarn-project/world-state/src/native/native_world_state.test.ts +++ b/yarn-project/world-state/src/native/native_world_state.test.ts @@ -1578,7 +1578,8 @@ describe('NativeWorldState', () => { const fork = await ws.fork(); await advanceState(fork); const siblingPathsBefore = await getSiblingPaths(fork); - await fork.createCheckpoint(); + const checkpointDepth = await fork.createCheckpoint(); + expect(checkpointDepth).toEqual(1); await compareState(fork, siblingPathsBefore, true); @@ -1593,7 +1594,7 @@ describe('NativeWorldState', () => { await compareState(fork, siblingPathsAfter, true); await compareState(fork, siblingPathsBefore, false); - await fork.commitAllCheckpoints(); + await fork.commitAllCheckpointsTo(checkpointDepth - 1); await compareState(fork, siblingPathsAfter, true); await compareState(fork, siblingPathsBefore, false); @@ -1604,7 +1605,8 @@ describe('NativeWorldState', () => { const fork = await ws.fork(); await advanceState(fork); const siblingPathsBefore = await getSiblingPaths(fork); - await fork.createCheckpoint(); + const checkpointDepth = await fork.createCheckpoint(); + expect(checkpointDepth).toEqual(1); await compareState(fork, siblingPathsBefore, true); @@ -1612,14 +1614,15 @@ describe('NativeWorldState', () => { let siblingPathsAfter: SiblingPath[] = []; for (let i = 0; i < numCommits; i++) { - await fork.createCheckpoint(); + const newCheckpointDepth = await fork.createCheckpoint(); + expect(newCheckpointDepth).toEqual(checkpointDepth + i + 1); siblingPathsAfter = await advanceState(fork); } await compareState(fork, siblingPathsAfter, true); await compareState(fork, siblingPathsBefore, false); - await fork.revertAllCheckpoints(); + await fork.revertAllCheckpointsTo(checkpointDepth - 1); await compareState(fork, siblingPathsAfter, false); await compareState(fork, siblingPathsBefore, true); @@ -1835,5 +1838,161 @@ describe('NativeWorldState', () => { await fork.close(); }); + + it('createCheckpoint returns depth', async () => { + const fork = await ws.fork(); + expect(await fork.createCheckpoint()).toBe(1); + expect(await fork.createCheckpoint()).toBe(2); + expect(await fork.createCheckpoint()).toBe(3); + await fork.close(); + }); + + it('can commit all to depth', async () => { + const fork = await ws.fork(); + + // Create 3 checkpoints with state changes between each + const initialPaths = await getSiblingPaths(fork); + + await fork.createCheckpoint(); // depth 1 + await advanceState(fork); + + await fork.createCheckpoint(); // depth 2 + await advanceState(fork); + + await fork.createCheckpoint(); // depth 3 + const afterDepth3Paths = await advanceState(fork); + + // Commit depths 3 and 2 into depth 1, leaving depth at 1 + await fork.commitAllCheckpointsTo(1); + + // State should reflect all changes + await compareState(fork, afterDepth3Paths, true); + + // Revert depth 1 — should go back to initial state + await fork.revertCheckpoint(); + await compareState(fork, initialPaths, true); + + await fork.close(); + }); + + it('can revert all to depth', async () => { + const fork = await ws.fork(); + + await fork.createCheckpoint(); // depth 1 + const afterDepth1Paths = await advanceState(fork); + + await fork.createCheckpoint(); // depth 2 + await advanceState(fork); + + await fork.createCheckpoint(); // depth 3 + await advanceState(fork); + + // Revert depths 3 and 2, leaving depth at 1 + await fork.revertAllCheckpointsTo(1); + + // Should be back to after depth 1 state + await compareState(fork, afterDepth1Paths, true); + + // Depth 1 still active — commit it + await fork.commitCheckpoint(); + await compareState(fork, afterDepth1Paths, true); + + await fork.close(); + }); + + it('revert to depth preserves lower checkpoints', async () => { + const fork = await ws.fork(); + + await fork.createCheckpoint(); // depth 1 + await advanceState(fork); + + await fork.createCheckpoint(); // depth 2 + await advanceState(fork); + + // Revert depth 2 only, leaving depth at 1 + await fork.revertAllCheckpointsTo(1); + + // Create new checkpoint at depth 2 with different changes + await fork.createCheckpoint(); // depth 2 again + const newDepth2Paths = await advanceState(fork); + + // Commit depth 2 + await fork.commitCheckpoint(); + + // Commit depth 1 + await fork.commitCheckpoint(); + + // Final state should include the new depth 2 changes + await compareState(fork, newDepth2Paths, true); + + await fork.close(); + }); + + it('commit all with depth 0 commits everything', async () => { + const fork = await ws.fork(); + + await fork.createCheckpoint(); // depth 1 + await advanceState(fork); + + await fork.createCheckpoint(); // depth 2 + const finalPaths = await advanceState(fork); + + // depth 0 commits all checkpoints + await fork.commitAllCheckpointsTo(0); + + // State should reflect all changes + await compareState(fork, finalPaths, true); + + await fork.close(); + }); + + it('revert all with depth 0 reverts everything', async () => { + const fork = await ws.fork(); + const initialPaths = await getSiblingPaths(fork); + + await fork.createCheckpoint(); // depth 1 + await advanceState(fork); + + await fork.createCheckpoint(); // depth 2 + await advanceState(fork); + + // depth 0 reverts all checkpoints + await fork.revertAllCheckpointsTo(0); + + // Should be back to initial state + await compareState(fork, initialPaths, true); + + await fork.close(); + }); + + it('depth is consistent across multiple checkpoint cycles', async () => { + const fork = await ws.fork(); + + // Create checkpoint depth 1 + expect(await fork.createCheckpoint()).toBe(1); + const afterDepth1Paths = await advanceState(fork); + + // Create checkpoint depth 2 + expect(await fork.createCheckpoint()).toBe(2); + await advanceState(fork); + + // Revert depth 2, leaving depth at 1 + await fork.revertAllCheckpointsTo(1); + await compareState(fork, afterDepth1Paths, true); + + // Create new depth 2 + expect(await fork.createCheckpoint()).toBe(2); + const newDepth2Paths = await advanceState(fork); + + // Commit depth 2 + await fork.commitCheckpoint(); + await compareState(fork, newDepth2Paths, true); + + // Commit depth 1 + await fork.commitCheckpoint(); + await compareState(fork, newDepth2Paths, true); + + await fork.close(); + }); }); });