Skip to content

Simplify chain switching#4785

Open
jackzhhuang wants to merge 3 commits intodual-verse-dagfrom
block_chain_switch
Open

Simplify chain switching#4785
jackzhhuang wants to merge 3 commits intodual-verse-dagfrom
block_chain_switch

Conversation

@jackzhhuang
Copy link
Contributor

@jackzhhuang jackzhhuang commented Jan 16, 2026

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Other information

Summary by CodeRabbit

  • New Features

    • Services now react to DAG block events: on-DAG updates can update the on-chain state root and apply new DAG heads.
  • Refactor

    • Centralized DAG block switching so the chain mutates its state in-place, streamlining state transitions and reducing redundant operations.
  • Bug Fixes

    • Removed an extra head-switch step during block processing to prevent unnecessary state churn on startup.
  • Tests

    • Updated tests to assert on the original chain object after state changes rather than on returned copies.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

This PR centralizes DAG-driven chain switches by adding BlockChain::switch_to_block (in-place state reconstruction) and changing select_dag_state to mutate the chain (Result<()>). Services and callers were updated to call select_dag_state on a mutable main chain (added get_main_mut()), and DAG events now trigger state-root updates and optional head application.

Changes

Cohort / File(s) Summary
Core DAG switching refactor
chain/src/chain.rs
Added pub fn switch_to_block(block_id: HashValue) -> Result<()> to rebuild accumulators, VM/state DBs, epoch and status in-place. Changed pub fn select_dag_state(&mut self, header: &BlockHeader) to return Result<()> and delegate final switching to switch_to_block.
Service layer mutable access
chain/service/src/chain_service.rs
Added pub fn get_main_mut(&mut self) -> &mut BlockChain. Replaced fork-and-assign logic for NewDagBlock handling with direct calls on get_main_mut() and select_dag_state.
State services subscribe to DAG events
state/service/src/service.rs, vm2/service/state/src/service.rs
Subscribed ChainStateService to NewDagBlock; added handlers that extract the executed block’s multi-state root and call change_root(...) (uses state_root1 or state_root2 as appropriate).
Sync: apply new head from DAG & switch_header simplified
sync/src/block_connector/write_block_chain.rs, sync/src/block_connector/block_connector_service.rs
Added apply_new_head_from_dag(executed_block: ExecutedBlock) -> Result<()>. switch_header now relies on select_dag_state side-effects and no longer captures/returns a new branch; block connector uses switch_header and, when headers match, calls apply_new_head_from_dag.
Tests updated for side-effects
chain/tests/test_select_dag_state.rs
Updated tests to stop capturing a returned chain from select_dag_state; assertions now inspect the mutated test_chain instance.

Sequence Diagram(s)

sequenceDiagram
  participant DAG as "DAG / EventBus"
  participant SyncSvc as "Sync Block Connector\n(write_block_chain)"
  participant ChainReader as "ChainReaderService\n(get_main_mut)"
  participant BlockChain as "BlockChain\n(select_dag_state / switch_to_block)"
  participant ChainState as "ChainStateService"
  participant VMState as "VM / State DB"

  DAG->>SyncSvc: NewDagBlock(msg with executed_block)
  SyncSvc->>ChainReader: switch_header(executed_block.header())
  ChainReader->>BlockChain: get_main_mut().select_dag_state(header)
  Note over BlockChain: select_dag_state calls switch_to_block when needed
  BlockChain->>VMState: rebuild accumulators & state DBs (in-place)
  BlockChain-->>ChainReader: return (Result<()>)
  alt header matches current head
    ChainReader->>SyncSvc: apply_new_head_from_dag(executed_block)
    SyncSvc-->>ChainReader: result
  end
  BlockChain->>ChainState: emit/update state root
  ChainState-->>VMState: change_root(state_root)
Loading

Note: rectangles/styles use default participant styling; component interactions and in-place switching are shown.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • sanlee42
  • welbon
  • simonjiao
  • nkysg

Poem

🐰 I hopped through DAGs and found a smoother track,
switch_to_block rebuilt the path, no need to hack.
get_main_mut gave me access, neat and clean,
NewDagBlock whispers, and states update unseen.
A tiny rabbit cheer for code now lean.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Simplify chain switching' accurately reflects the main objective of the changeset, which centralizes and refactors DAG switching logic across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a76a4576dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 177 to 179
pub fn switch_header(&mut self, header: &BlockHeader) -> Result<BlockHeader> {
let new_branch = self.main.select_dag_state(header)?; // 1
self.select_head(new_branch)?;
self.main.select_dag_state(header)?;
self.update_startup_info(&self.main.current_header())?;

Choose a reason for hiding this comment

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

P1 Badge Preserve head selection side effects in switch_header

switch_header now only mutates self.main via select_dag_state and immediately updates startup info, but it no longer calls select_head/do_new_head. That removes the total-difficulty check and, more importantly, skips reorg handling (txpool updates, metrics, and NewHeadBlock/NewBranch broadcasts). When a NewDagBlock arrives (handled in BlockConnectorService), the chain will silently switch to the selected block without notifying subscribers or updating the txpool, leaving downstream components out of sync after a head change or reorg.

Useful? React with 👍 / 👎.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Simplify the selecting block chain header logic in ChainReaderService

1 participant