Skip to content

feat: FCU unwind: properly reorg in-memory canonical state and update latest block#17938

Merged
mattsse merged 5 commits intoparadigmxyz:mainfrom
pycckuu:igor/feat/force-fcu-return-rebased-with-styles
Aug 26, 2025
Merged

feat: FCU unwind: properly reorg in-memory canonical state and update latest block#17938
mattsse merged 5 commits intoparadigmxyz:mainfrom
pycckuu:igor/feat/force-fcu-return-rebased-with-styles

Conversation

@pycckuu
Copy link
Contributor

@pycckuu pycckuu commented Aug 19, 2025

Fix FCU when new head is an ancestor (unwind). Previously we only updated the header, leaving in-memory canonical state stale, causing tx validation issues (e.g., "nonce too low"). Now we detect unwind, reorg in-memory state (remove old blocks, load ancestor), and keep TreeState and CanonicalInMemoryState in sync.

Code Changes

  • tree/mod.rs:
  • Add: log_chain_update_type, handle_chain_unwind, collect_blocks_for_removal, apply_canonical_ancestor_via_reorg, handle_chain_advance_or_same_height, ensure_block_in_memory.
  • Replace header-only update with:
    self.log_chain_update_type(...);
    self.state.tree_state.set_canonical_head(canonical_header.num_hash());
    if new_head_number < current_head_number {
        self.handle_chain_unwind(current_head_number, canonical_header)
    } else {
        self.handle_chain_advance_or_same_height(canonical_header)
    }
  • Reorg on unwind:
    self.canonical_in_memory_state.update_chain(NewCanonicalChain::Reorg { new, old });
    self.canonical_in_memory_state.set_canonical_head(canonical_header.clone());
  • Advance/same-height: ensure block committed into memory if missing.
  • Interim method update_latest_block_to_canonical_ancestor added, then superseded by full unwind handling.
  • tree/tests.rs:
  • New test test_fcu_with_canonical_ancestor_updates_latest_block asserting both TreeState and in-memory head match the ancestor after FCU.

Reason for Changes

Correctness: prevent stale in-memory state on reorgs that led to txpool/state provider errors.

Impact of Changes

  • Correct state after unwinds; eliminate nonce/state desync errors.
  • Low overhead; clearer logging.

Test Plan

  • Run: cargo test -p engine_tree test_fcu_with_canonical_ancestor_updates_latest_block.

How the reviewer should test the fix

  • Unwind: head=4 → FCU to 2; verify removal of [3,4] and head=2 in both states.
  • Advance: head=2 → FCU to 3; verify block 3 committed and head updated.
  • Check logs for correct classification.

Other useful info

If ancestor block missing from storage, we warn and still update header to avoid desync.

Closes #17798

mattsse and others added 2 commits August 19, 2025 13:54
When a `forkchoiceUpdated` call points to a canonical ancestor of the
current head (an unwind scenario), the engine's internal state for the
"latest" block was not being reverted to match.

This caused a state desynchronization where components like the
transaction pool would operate on a stale, more advanced block state.
This could lead to errors, such as "nonce too low", when validating
new transactions against the incorrect (post-reorg) state.

This commit introduces the `update_latest_block_to_canonical_ancestor`
method, which is now called during FCU processing. This function ensures
that both the `TreeState` and the `CanonicalInMemoryState` are correctly
updated to the new head, resolving the state inconsistency after a reorg.
@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Aug 19, 2025
@pycckuu pycckuu changed the title Igor/feat/force fcu return rebased with styles Fix FCU unwind: properly reorg in-memory canonical state and update latest block Aug 19, 2025
@pycckuu pycckuu force-pushed the igor/feat/force-fcu-return-rebased-with-styles branch 2 times, most recently from 031e683 to 27c8e17 Compare August 19, 2025 12:00
@pycckuu pycckuu marked this pull request as ready for review August 19, 2025 12:02
@pycckuu
Copy link
Contributor Author

pycckuu commented Aug 19, 2025

@mattsse I've rebased on the latest main branch changes. Could you please review? I'd recommend checking by commit since one commit addresses linting issues with cargo +nightly fmt and touches many files.

@mediocregopher
Copy link
Member

Hi @pycckuu, can you leave out the linting changes which aren't related to the fix being submitted? They make it impossible to review this.

@pycckuu
Copy link
Contributor Author

pycckuu commented Aug 19, 2025

Hi @pycckuu, can you leave out the linting changes which aren't related to the fix being submitted? They make it impossible to review this.

I can definitely do that, but as mentioned earlier, reviewing commit-by-commit would be much more manageable. I'm also unsure how to handle linting issues that originate from the main branch and code I haven't modified.

@pycckuu pycckuu force-pushed the igor/feat/force-fcu-return-rebased-with-styles branch from 27c8e17 to 33672f4 Compare August 19, 2025 16:08
@pycckuu
Copy link
Contributor Author

pycckuu commented Aug 19, 2025

@mediocregopher I've removed the styling commit. Could you please review it? Thanks in advance.

Copy link
Member

@mediocregopher mediocregopher left a comment

Choose a reason for hiding this comment

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

One nit and one question, will want @mattsse to look at it as well

// Update the latest block state to reflect the canonical ancestor.
// This ensures that state providers and the transaction pool operate with
// the correct chain state after forkchoice update processing.
self.update_latest_block_to_canonical_ancestor(&canonical_header)?;
Copy link
Member

Choose a reason for hiding this comment

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

Is it correct that this is being called after the process_payload_attributes if-branch? If attrs.is_some() then this line never gets hit.

Copy link
Contributor Author

@pycckuu pycckuu Aug 20, 2025

Choose a reason for hiding this comment

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

I suppose, the assumption here is:

  • Without attrs: We're just updating the head, so the state needs to be fixed
  • With attrs: We're building the payload and assuming the state is already correct

The placement of the new code was suggested by @mattsse in 9e8735d

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Aug 20, 2025
@pycckuu pycckuu force-pushed the igor/feat/force-fcu-return-rebased-with-styles branch from 33672f4 to b79881f Compare August 20, 2025 18:01
The previous implementation only updated the canonical head header on a
forkchoice update. This was insufficient for handling reorgs where the
new head is an ancestor of the current head (an unwind). This
discrepancy could lead to a stale in-memory state.

When the state provider attempted to access account information, it
would fall back to the stale database state, causing transaction
validation failures such as "nonce too low" errors.

This commit refactors the logic to correctly handle unwinds. It now
detects when the new head number is lower than the current one and
treats it as a reorg. It collects the now-invalid blocks from the
in-memory state and updates the chain by removing them and loading the
new canonical ancestor. This ensures the in-memory state accurately
reflects the canonical chain after a reorg.
@pycckuu pycckuu force-pushed the igor/feat/force-fcu-return-rebased-with-styles branch from b79881f to 15e077a Compare August 20, 2025 18:05
Copy link
Member

@mediocregopher mediocregopher left a comment

Choose a reason for hiding this comment

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

lgtm, pending matt's review

@pycckuu
Copy link
Contributor Author

pycckuu commented Aug 25, 2025

gentle ping @mattsse!

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

I'm okay with this because this code path is only reachable if explicitly opt-in.

@mattsse mattsse changed the title Fix FCU unwind: properly reorg in-memory canonical state and update latest block feat: FCU unwind: properly reorg in-memory canonical state and update latest block Aug 26, 2025
@mattsse mattsse added this pull request to the merge queue Aug 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 26, 2025
@mattsse mattsse enabled auto-merge August 26, 2025 16:41
@mattsse mattsse added this pull request to the merge queue Aug 26, 2025
Merged via the queue into paradigmxyz:main with commit 92743a0 Aug 26, 2025
41 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Aug 26, 2025
lwedge99 pushed a commit to sentioxyz/reth that referenced this pull request Sep 16, 2025
… latest block (paradigmxyz#17938)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
joshieDo added a commit that referenced this pull request Jan 28, 2026
When PruneMode::Full is used for receipts, the pruner would prune all
receipts up to the tip block immediately after persistence. This causes
a race condition during chain reorganizations:

1. Block A at height N is persisted to disk
2. Pruner runs and prunes A's receipts
3. Block B at height N arrives (reorg)
4. FCU tries to make B canonical
5. on_new_head calls canonical_block_by_hash(A) to walk back old chain
6. canonical_block_by_hash tries to reconstruct ExecutedBlock from disk
7. get_state() fails with 'no receipt found' because receipts were pruned

This fix adds MINIMUM_RECEIPTS_DISTANCE (64 blocks) to ensure receipts
and bodies are retained long enough to handle any potential reorgs.

Fixes a regression introduced by PR #17938 which added canonical_block_by_hash
that assumes ExecutedBlock can always be reconstructed from disk.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

After FCU updates canonical head: reinjected txs miss the immediate block; returns “nonce too low” on immediate resubmission

3 participants