Skip to content
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

Reorgs not possible on op-reth #8277

Closed
joshieDo opened this issue May 15, 2024 · 1 comment
Closed

Reorgs not possible on op-reth #8277

joshieDo opened this issue May 15, 2024 · 1 comment
Labels
A-consensus Related to the consensus engine A-op-reth Related to Optimism and op-reth C-bug An unexpected or incorrect behavior

Comments

@joshieDo
Copy link
Collaborator

Describe the feature

Any block that gets sent from op-node to op-reth is actually built by op-reth. Including reorg blocks. This means that if there would be a 1 block reorg, we would receive a FCU with head to block N-1 with attributes to build the new block N.

If this would be mainnet, since N-1 is an ancestor, we would just skip the payload job according to mainnet execution-api specs:

Client software MAY skip an update of the forkchoice state and MUST NOT begin a payload build process if forkchoiceState.headBlockHash references a VALID ancestor of the head of canonical chain, i.e. the ancestor passed [payload validation](https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#payload-validation) process and deemed VALID. In the case of such an event, client software MUST return {payloadStatus: {status: VALID, latestValidHash: forkchoiceState.headBlockHash, validationError: null}, payloadId: null}.

In case of op-reth, we update the head , however we don't rewind the state, leading to the payload job being done against block N state instead of block N - 1 state

/// Returns `true` if the head needs to be updated.
fn on_head_already_canonical(
&self,
header: &SealedHeader,
attrs: &mut Option<EngineT::PayloadAttributes>,
) -> bool {
// On Optimism, the proposers are allowed to reorg their own chain at will.
#[cfg(feature = "optimism")]
if self.blockchain.chain_spec().is_optimism() {
debug!(
target: "consensus::engine",
fcu_head_num=?header.number,
current_head_num=?self.blockchain.canonical_tip().number,
"[Optimism] Allowing beacon reorg to old head"
);
return true
}

if should_update_head {
let head = outcome.header();
let _ = self.update_head(head.clone());
self.listeners.notify(BeaconConsensusEngineEvent::CanonicalChainCommitted(
Box::new(head.clone()),
elapsed,
));
}
// Validate that the forkchoice state is consistent.
let on_updated = if let Some(invalid_fcu_response) =
self.ensure_consistent_forkchoice_state(state)?
{
trace!(target: "consensus::engine", ?state, "Forkchoice state is inconsistent");
invalid_fcu_response
} else if let Some(attrs) = attrs {
// the CL requested to build a new payload on top of this new VALID head
let head = outcome.into_header().unseal();
self.process_payload_attributes(attrs, head, state)
} else {

Additional context

No response

@joshieDo joshieDo added C-bug An unexpected or incorrect behavior A-consensus Related to the consensus engine A-op-reth Related to Optimism and op-reth labels May 15, 2024
@joshieDo
Copy link
Collaborator Author

closing since it's wrong. we do call state_by_block_hash when building the payload

@joshieDo joshieDo closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine A-op-reth Related to Optimism and op-reth C-bug An unexpected or incorrect behavior
Projects
Status: Done
Development

No branches or pull requests

1 participant