Skip to content

triedb: add namespace state merge path - #2449

Draft
zander-xyz wants to merge 1 commit into
alee/namespace-read-pathfrom
alee/namespace-merge
Draft

triedb: add namespace state merge path#2449
zander-xyz wants to merge 1 commit into
alee/namespace-read-pathfrom
alee/namespace-merge

Conversation

@zander-xyz

Copy link
Copy Markdown
Contributor

No description provided.

@zander-xyz
zander-xyz force-pushed the alee/namespace-merge branch from 64c33f0 to 30809d6 Compare July 20, 2026 15:00
@zander-xyz
zander-xyz requested a review from Copilot July 20, 2026 15:00
@zander-xyz

Copy link
Copy Markdown
Contributor Author

@claude review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Ethereum execution “state” plumbing to support namespaced state deltas in the BlockState merge/validation path (used by TrieDB namespace reads), and adds a regression test that exercises reads + merges across two namespaces.

Changes:

  • Add State::get_namespace() accessor to expose the state’s optional namespace.
  • Update BlockState::{can_merge,merge} to select the correct StateDeltas map (global vs per-namespace) when validating/merging.
  • Extend the on-disk TrieDB test to cover namespaced merges and stale-state rejection.

Verdict: NEEDS CHANGES

Reviewed changes

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

File Description
category/execution/ethereum/state3/state.hpp Exposes the optional namespace from State for downstream merge logic.
category/execution/ethereum/state2/block_state.cpp Routes merge/validation to namespace-specific deltas when State is namespaced.
category/execution/ethereum/db/test/test_db.cpp Expands namespace test to validate merge behavior and stale-state rejection.

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

Comment thread category/execution/ethereum/state2/block_state.cpp
Comment thread category/execution/ethereum/state2/block_state.cpp

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes cleanly extend BlockState::can_merge and BlockState::merge to route to the correct StateDeltas based on the State's namespace, mirroring the pattern already used in read_account / read_storage. The test covers namespace-scoped merges, cross-namespace isolation, and rejection of a stale state after an intervening merge.

A few very minor style/redundancy notes, none blocking:

  • MONAD_ASSERT(state_) at the top of can_merge and the inner MONAD_ASSERT(deltas) in the loop are redundant now that the namespace branch already establishes deltas (and state_ is a construction-time invariant).
  • merge uses NamespacedStateDeltas::accessor (write lock) but only needs a const_accessor since it mutates only the pointed-to StateDeltas, not the outer map.
  • The raw deltas pointer outlives its accessor lock in both functions. This is safe today because ns_state_ has no erase code path, but worth keeping in mind if erasure is ever added.

Verdict: CORRECT

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants