triedb: add native namespace state delta commit path - #2453
Conversation
a61a176 to
40cceae
Compare
|
@claude review |
There was a problem hiding this comment.
Pull request overview
This PR extends the monad::Db interface with a native “namespace-scoped state delta commit” operation and wires it through the primary DB implementations, statesync server context, and migration dual-write path. It also adds an end-to-end test that validates which encoding (slot vs page) is considered canonical for namespace state roots across the migration fork boundary.
Changes:
- Add
Db::commit_namespace_state_deltas(...)as a required virtual API and implement/override it acrossTrieDb,PartialTrieDb,TrieRODb, andmonad_statesync_server_context. - Introduce
commit_native_namespace_state_deltas<Traits>(...)in the migration commit path to dual-write namespace deltas and return the canonical encoding’s roots depending ontraits::mip_8_active(). - Add a migration test asserting namespace state root canonicalization pre-/post-fork while ensuring both DBs maintain consistent readable namespace state.
Verdict: CORRECT
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| category/execution/ethereum/db/db.hpp | Adds the new pure-virtual commit_namespace_state_deltas API to the core DB interface. |
| category/execution/ethereum/db/trie_db.hpp | Marks TrieDb::commit_namespace_state_deltas as an override of the new interface method. |
| category/execution/ethereum/db/trie_rodb.hpp | Adds an aborting override for namespace commits on the read-only DB wrapper. |
| category/execution/ethereum/db/partial_trie_db.hpp | Declares the new override in the witness/partial DB implementation. |
| category/execution/ethereum/db/partial_trie_db.cpp | Implements the override as a no-op (asserting deltas are empty) for witness contexts. |
| category/statesync/statesync_server_context.hpp | Extends the statesync server DB context to override the new interface method. |
| category/statesync/statesync_server_context.cpp | Delegates namespace-delta commits to the underlying read-write TrieDb. |
| category/execution/monad/db/commit_block_migration.hpp | Declares commit_native_namespace_state_deltas for the migration dual-write path. |
| category/execution/monad/db/commit_block_migration.cpp | Implements commit_native_namespace_state_deltas and explicitly instantiates it for Monad traits. |
| category/execution/monad/db/test_page_storage_migration.cpp | Adds an end-to-end test validating canonical namespace root selection across the migration fork boundary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Reviewed the diff. The new commit_native_namespace_state_deltas mirrors commit_block's dual-write structure closely: the same is_page_encoded/mip_8_active invariant assertions, canonical-first ordering, and single-db/dual-db split. The pure-virtual commit_namespace_state_deltas on Db is properly overridden in TrieDb, PartialTrieDb (asserts empty, since witnesses don't carry namespace state), TrieRODb (aborts), and monad_statesync_server_context (forwards to rw). Trait instantiation via EXPLICIT_MONAD_TRAITS is correct.
A couple of observations that didn't rise to inline findings:
- The canonical-first ordering in the dual-write path is stricter than needed — unlike
commit_block, there's nopopulate_header_fnreading from the canonical db, so either order would produce the same roots. Matchingcommit_block's pattern is fine for consistency, but a one-line comment would save a future reader some head-scratching about "why does the order matter here." monad_statesync_server_context::commitcallson_commit(...)before forwarding to record proposed deletions for statesync clients;commit_namespace_state_deltasdoes not. GivenDeletionhas no namespace field, namespace-aware statesync is clearly follow-up work, but worth tracking so it doesn't get lost.
Verdict: CORRECT
🤖 Generated with Claude Code
No description provided.