Fix traverse_blocking to call up() correctly on failure path

traverse_blocking was not calling up() to restore the TraverseMachine
state when failing. This ensures the state machine is properly restored
on both success and failure paths. Asserts that the state machine is at
the top most level after blocking traverse.

Author: Chen-Yifan

category/mpt/test/db_test.cpp

@@ -873,8 +873,7 @@ TEST(DbTest, history_length_adjustment_never_under_min)
     remove(dbname);
 }
 
-TEST_F(
-    OnDiskDbWithFileFixture, read_only_db_traverse_fail_upon_version_expiration)
+TEST_F(OnDiskDbWithFileFixture, read_only_db_traverse_as_version_expire)
 {
     struct TraverseMachinePruneHistory final : public TraverseMachine
     {
@@ -927,6 +926,12 @@ TEST_F(
         {
             return std::make_unique<TraverseMachinePruneHistory>(*this);
         }
+
+        void reset()
+        {
+            path = Nibbles{};
+            has_done_callback = false;
+        }
     };
 
     EXPECT_EQ(db.get_history_length(), MPT_TEST_HISTORY_LENGTH);
@@ -942,24 +947,51 @@ TEST_F(
         root = db.upsert(std::move(root), std::move(ls), version);
     };
 
-    while (version < MPT_TEST_HISTORY_LENGTH - 1) {
+    while (version < MPT_TEST_HISTORY_LENGTH) {
         upsert_once();
         ++version;
     }
     // traverse
-    AsyncIOContext io_ctx{ReadOnlyOnDiskDbConfig{.dbname_paths = {dbname}}};
-    Db ro_db{io_ctx};
     TraverseMachinePruneHistory traverse_machine{upsert_once};
-    auto const read_version = ro_db.get_earliest_version();
-    ASSERT_EQ(read_version, 0);
-    NodeCursor const root_cursor{ro_db.load_root_for_version(read_version)};
-    ASSERT_EQ(
-        ro_db.traverse(root_cursor, traverse_machine, read_version), true);
-    EXPECT_EQ(ro_db.get_earliest_version(), read_version);
-    ++version;
-    ASSERT_EQ(
-        ro_db.traverse(root_cursor, traverse_machine, read_version), false);
-    EXPECT_GT(ro_db.get_earliest_version(), read_version);
+
+    // Helper to test both traverse APIs
+    auto test_traverse = [&](auto &db, auto traverse_fn) {
+        traverse_machine.reset();
+        version = db.get_latest_version();
+        auto const latest_version = db.get_latest_version();
+        auto const earliest_version = db.get_earliest_version();
+        auto root_cursor = db.find({}, latest_version).value();
+        ASSERT_EQ(
+            traverse_fn(root_cursor, traverse_machine, latest_version), true);
+        EXPECT_EQ(traverse_machine.path.nibble_size(), 0u);
+        EXPECT_EQ(db.get_earliest_version(), earliest_version);
+
+        // traverse earliest will erase earliest version
+        root_cursor = db.find({}, earliest_version).value();
+        ++version;
+        traverse_machine.reset();
+        ASSERT_EQ(
+            traverse_fn(root_cursor, traverse_machine, earliest_version),
+            false);
+        EXPECT_GT(db.get_earliest_version(), earliest_version);
+        EXPECT_EQ(traverse_machine.path.nibble_size(), 0u);
+    };
+
+    {
+        AsyncIOContext io_ctx{ReadOnlyOnDiskDbConfig{.dbname_paths = {dbname}}};
+        Db ro_db{io_ctx};
+        // Test both APIs
+        test_traverse(ro_db, [&](auto &&...args) {
+            return ro_db.traverse_blocking(args...);
+        });
+        test_traverse(
+            ro_db, [&](auto &&...args) { return ro_db.traverse(args...); });
+    }
+    {
+        RODb ro_db(ReadOnlyOnDiskDbConfig{.dbname_paths = {dbname}});
+        test_traverse(
+            ro_db, [&](auto &&...args) { return ro_db.traverse(args...); });
+    }
 }
 
 TEST_F(OnDiskDbWithFileFixture, benchmark_blocking_parallel_traverse)
@@ -1568,6 +1600,7 @@ TYPED_TEST(DbTraverseTest, trimmed_traverse)
     // Trimmed traversal
     struct TrimmedTraverse : public TraverseMachine
     {
+        unsigned curr_level{0};
         size_t &num_leaves;
 
         TrimmedTraverse(size_t &num_leaves)
@@ -1584,10 +1617,14 @@ TYPED_TEST(DbTraverseTest, trimmed_traverse)
             if (node.has_value()) {
                 ++num_leaves;
             }
+            ++curr_level;
             return true;
         }
 
-        virtual void up(unsigned char const, Node const &) override {}
+        virtual void up(unsigned char const, Node const &) override
+        {
+            --curr_level;
+        }
 
         virtual std::unique_ptr<TraverseMachine> clone() const override
         {
@@ -1610,13 +1647,15 @@ TYPED_TEST(DbTraverseTest, trimmed_traverse)
         TrimmedTraverse traverse{num_leaves};
         ASSERT_TRUE(
             this->db.traverse(res_cursor.value(), traverse, this->block_id));
+        ASSERT_EQ(traverse.curr_level, 0);
         EXPECT_EQ(num_leaves, 2);
     }
     {
         size_t num_leaves = 0;
         TrimmedTraverse traverse{num_leaves};
         ASSERT_TRUE(this->db.traverse_blocking(
             res_cursor.value(), traverse, this->block_id));
+        ASSERT_EQ(traverse.curr_level, 0);
         EXPECT_EQ(num_leaves, 2);
     }
 }

category/mpt/traverse.hpp

@@ -74,20 +74,29 @@ namespace detail
             --traverse.level;
             return true;
         }
-        for (auto const [idx, branch] : NodeChildrenRange(node.mask)) {
-            if (traverse.should_visit(node, branch)) {
+        for (auto const [idx, next_branch] : NodeChildrenRange(node.mask)) {
+            if (traverse.should_visit(node, next_branch)) {
                 if (Node::SharedPtr const &next = node.next(idx);
                     next != nullptr) {
-                    preorder_traverse_blocking_impl(
-                        aux, branch, *next, traverse, version);
+                    if (!preorder_traverse_blocking_impl(
+                            aux, next_branch, *next, traverse, version)) {
+                        --traverse.level;
+                        traverse.up(branch, node);
+                        return false;
+                    }
                     continue;
                 }
                 MONAD_ASSERT(aux.is_on_disk());
                 Node::UniquePtr next_node_ondisk =
                     read_node_blocking(aux, node.fnext(idx), version);
-                if (!next_node_ondisk ||
-                    !preorder_traverse_blocking_impl(
-                        aux, branch, *next_node_ondisk, traverse, version)) {
+                if (!next_node_ondisk || !preorder_traverse_blocking_impl(
+                                             aux,
+                                             next_branch,
+                                             *next_node_ondisk,
+                                             traverse,
+                                             version)) {
+                    --traverse.level;
+                    traverse.up(branch, node);
                     return false;
                 }
             }
@@ -368,8 +377,10 @@ inline bool preorder_traverse_blocking(
     UpdateAuxImpl &aux, Node const &node, TraverseMachine &traverse,
     uint64_t const version)
 {
-    return detail::preorder_traverse_blocking_impl(
+    auto const ret = detail::preorder_traverse_blocking_impl(
         aux, INVALID_BRANCH, node, traverse, version);
+    MONAD_ASSERT(traverse.level == 0);
+    return ret;
 }
 
 inline bool preorder_traverse_ondisk(