Cache recent roots to prevent trie wipe on out-of-order commits

[Chen-Yifan]

There's a 10% performance regression introduced by the first commit.

[Copilot]

Pull request overview

This PR aims to prevent MPT “trie wipe” behavior during out-of-order commits by keeping recent trie roots alive and avoiding destructive child-pointer transfers when traversing/copying tries.

Changes:

• Replace several move_next(...) calls with next(...) in MPT update/expire/compact flows to avoid mutating existing nodes while traversing.
• Add a small fixed-size ring buffer to cache recent trie roots in TrieDb (on-disk mode) and route root loads through it.
• Add unit tests covering RootRingBuffer behavior (sequential insert, eviction, gaps, wraparound).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
category/mpt/trie.cpp	Switch child access from destructive moves to non-destructive reads in several recursion paths.
category/mpt/copy_trie.cpp	Avoid moving child pointers when constructing nodes during trie copy.
category/execution/ethereum/db/trie_db.hpp	Introduce RootRingBuffer template and add root_cache_ to TrieDb.
category/execution/ethereum/db/trie_db.cpp	Add load_root() helper and insert committed/finalized roots into the cache.
category/execution/ethereum/db/test/test_root_ring_buffer.cpp	Add tests validating ring-buffer caching semantics.

Comments suppressed due to low confidence (2)

category/execution/ethereum/db/trie_db.hpp:40

• RootRingBuffer uses MONAD_ASSERT but this header doesn’t include the header that defines it. This makes trie_db.hpp non-self-contained and can break compilation for translation units that include it directly (e.g. the new unit test). Include the appropriate assert header (or avoid the macro in this header).

#include <nlohmann/json.hpp>

#include <array>
#include <deque>
#include <istream>
#include <memory>
#include <optional>

category/mpt/trie.cpp:368

• In the single-child coalescing path, switching from move_next() to next() means single_child remains referenced by orig. But the subsequent make_node(*single_child, ...) call moves child pointers out of single_child (see make_node(Node& from, ...)), which will mutate the cached trie node that orig still points to and can effectively “wipe” cached children. This path likely still needs to transfer ownership (keep move_next() here) or use a non-destructive clone that copies child pointers instead of moving them.

    if (number_of_children == 1 && !orig->has_value()) {
        auto const child_branch = static_cast<uint8_t>(std::countr_zero(mask));
        auto const child_index = orig->to_child_index(child_branch);
        auto single_child = orig->next(child_index);
        if (!single_child) {
            node_receiver_t recv{
                [aux = &aux,
                 sm = sm.clone(),
                 tnode = std::move(tnode),

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.