Skip to content

Advertise finalized head as LIB on engine-API path#5

Open
sduchesneau wants to merge 2 commits into
firehose/2.xfrom
firehose/fix-lib-finalized
Open

Advertise finalized head as LIB on engine-API path#5
sduchesneau wants to merge 2 commits into
firehose/2.xfrom
firehose/fix-lib-finalized

Conversation

@sduchesneau

Copy link
Copy Markdown

Bug

LIB (last irreversible / finalized block) was never advanced for the Firehose-instrumented op-reth running the live engine-API flow.

The cloned OpFirehoseEngineValidator (op-reth's own engine-validator clone, in crates/firehose/src/engine_validator.rs) started the block tracer with finalized = None:

let tracer = FirehoseBlockTracer::start::<N>(&sealed, None);

so every emitted block carried no finalized reference and downstream LIB never moved.

Fix

Read the finalized head from the node provider and pass it as the block's FinalizedBlockRef:

let finalized = self.provider.finalized_block_num_hash().ok().flatten().map(|num_hash| {
    firehose_tracer::types::FinalizedBlockRef { number: num_hash.number, hash: Some(num_hash.hash) }
});
let tracer = FirehoseBlockTracer::start::<N>(&sealed, finalized);

This mirrors the reth fork's runner.rs (ExEx path), which already does to_finalized_ref(ctx.provider().finalized_block_num_hash()). The op-reth engine-API clone was the one place still passing None. Requires a BlockIdReader bound on the provider; the concrete Node::Provider (BlockchainProvider, with the live forkchoice-tracked finalized) satisfies it.

Validation

  • cargo check -p reth-optimism-firehose / -p reth-optimism-node
  • cargo build -p op-reth --bin op-reth

🤖 Generated with Claude Code

The cloned OpFirehoseEngineValidator started the Firehose block tracer with
`finalized = None`, so live engine-API blocks carried no finalized reference and
downstream LIB (last irreversible block) never advanced.

Read the finalized head from the node provider via
`finalized_block_num_hash()` (adding a `BlockIdReader` bound) and pass it as the
block's `FinalizedBlockRef`, mirroring the reth fork's `runner.rs` ExEx path.

@UlysseCorbeil UlysseCorbeil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm

Cold builds took ~32m (no layer cache; maxperf LTO). Cache the cargo-chef
dependency layer across runs (type=gha) and use the fast-build profile for PR
builds, keeping maxperf for branch/tag releases. Mirrors build-images.yaml.
@sduchesneau

Copy link
Copy Markdown
Author

DO NOT MERGE YET: it does not fix the LIB. I should make PR-builds faster though

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