Skip to content

wip:feat: update chain on fcu unwind #17857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Aug 13, 2025

ref #17798

just a smol comment re what would need to be done here in order to support this usecase that isn't allowed by the ethereum spec, but I'm unsure if we should even support unwinding the chain like this.
the intended use case for this was originally so that op can build an alternative chain based on a canonical ancestor block. the limitation with how we currently handle this is that this doesn't reset the node's LATEST state so until the new alternative block is applied the current head block remains the LATEST.

we effectively would need to do step 3 here as well, but this becomes slightly more difficult if a child block of the new head is already committed to disk

// 3. ensure we can apply a new chain update for the head block
if let Some(chain_update) = self.on_new_head(state.head_block_hash)? {
let tip = chain_update.tip().clone_sealed_header();
self.on_canonical_chain_update(chain_update);
// update the safe and finalized blocks and ensure their values are valid
if let Err(outcome) = self.ensure_consistent_forkchoice_state(state) {
// safe or finalized hashes are invalid
return Ok(TreeOutcome::new(outcome))
}
if let Some(attr) = attrs {
let updated = self.process_payload_attributes(attr, &tip, state, version);
return Ok(TreeOutcome::new(updated))
}
return Ok(valid_outcome(state.head_block_hash))
}

Copy link
Contributor

Your PR title doesn't follow the Conventional Commit guidelines.

Example of valid titles:

  • feat: add new user login
  • fix: correct button size
  • docs: update README

Usage:

  • feat: Introduces a new feature
  • fix: Patches a bug
  • chore: General maintenance tasks or updates
  • test: Adding new tests or modifying existing tests
  • bench: Adding new benchmarks or modifying existing benchmarks
  • perf: Performance improvements
  • refactor: Changes to improve code structure
  • docs: Documentation updates
  • ci: Changes to CI/CD configurations
  • revert: Reverts a previously merged PR
  • deps: Updates dependencies

Breaking Changes

Breaking changes are noted by using an exclamation mark. For example:

  • feat!: changed the API
  • chore(node)!: Removed unused public function

Help

For more information, follow the guidelines here: https://www.conventionalcommits.org/en/v1.0.0/

@mattsse mattsse marked this pull request as draft August 13, 2025 15:45
Copy link

codspeed-hq bot commented Aug 13, 2025

CodSpeed Performance Report

Merging #17857 will improve performances by 10.99%

Comparing matt/add-note-about-fcu-unwind (f3a3fff) with main (ee8c893)

Summary

⚡ 1 improvements
✅ 76 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
hash builder[init size 10000 | update size 100 | num updates 1] 9.6 ms 8.7 ms +10.99%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

1 participant