EVM: Set FinalizedHash correctly in forkChoiceUpdatedV3 call in ExecuteTxs

In #2394, in order to have sequencer restarts work well, we modified the first call to forkChoiceUpdatedV3 in the ExecuteTxs EVM implementation to assign the previous block hash to HeadBlockHash, SafeBlockHash, and FinalizedBlockHash.

We should revisit this and take one or more of the following approaches:

Set the FinalizedBlockHash based on the DA Included Height from the Rollkit Store. For the Head and Safe block hashes, use the Rollkit height which should be the same as the reth block height but it might be good to actually sanity check that on startup and set them in engineClient correctly on start.
Persist the current versions of head, safe, and finalizedBlockHashes somewhere so on restarts they can be loaded back easily.

The concrete consequences of setting finalizedHash incorrectly are unknown currently but it'll likely be at least an issue in indexers and block explorers showing inconsistent finalized block hashes for a small period of time.

@claude

@claude can you propose a solution to this issue

@tac0turtle

Claude finished @tac0turtle's task —— View job

EVM FinalizedHash Issue Analysis & Solution Proposal

I've analyzed the codebase and identified the issue with FinalizedHash setting in the EVM's forkChoiceUpdatedV3 call. Here's my comprehensive solution proposal:

Todo List:

Understand the current codebase structure and find EVM-related code
Locate the problematic forkChoiceUpdatedV3 implementation
Research DA layer integration and block height tracking
Analyze block hash persistence options
Propose a concrete solution with implementation details

Problem Analysis

The issue is in /execution/evm/execution.go:162-166 where the first forkChoiceUpdatedV3 call incorrectly sets all three block hashes to the previous block hash:

args := engine.ForkchoiceStateV1{
    HeadBlockHash:      prevBlockHash,
    SafeBlockHash:      prevBlockHash,
    FinalizedBlockHash: prevBlockHash,  // ❌ This is incorrect
}

This was a temporary fix for sequencer restarts but causes indexers and block explorers to show inconsistent finalized block hashes.

Root Cause

Incorrect Finalized Hash: Setting FinalizedBlockHash to prevBlockHash doesn't reflect the actual finalized state
Missing DA Integration: The finalized hash should correspond to the DA-included height, not the previous block
No State Persistence: Block hash states aren't persisted across restarts

Proposed Solution

I recommend Approach #1 from the issue description with enhancements:

1. Use DA Included Height for FinalizedBlockHash

The codebase already has a robust DA inclusion tracking system:

DAIncludedHeight is tracked in the store (pkg/store/keys.go:16)
incrementDAIncludedHeight() in block/da_includer.go:54 calls SetFinal() when blocks are DA-included
This provides the correct finalized block height

2. Enhanced Implementation Plan

Phase 1: Fix FinalizedBlockHash Logic

// In ExecuteTxs, replace lines 162-166 with:
func (c *EngineClient) getForkChoiceState(ctx context.Context, prevBlockHash common.Hash) (engine.ForkchoiceStateV1, error) {
    c.mu.Lock()
    defer c.mu.Unlock()
    
    // Use the stored finalized hash, fallback to genesis if not set
    finalizedHash := c.currentFinalizedBlockHash
    if finalizedHash == (common.Hash{}) {
        finalizedHash = c.genesisHash
    }
    
    return engine.ForkchoiceStateV1{
        HeadBlockHash:      prevBlockHash,
        SafeBlockHash:      prevBlockHash,
        FinalizedBlockHash: finalizedHash,
    }, nil
}

Phase 2: Integrate with DA Height System

// Add method to sync EVM state with DA-included height
func (c *EngineClient) SyncWithDAHeight(ctx context.Context, daIncludedHeight uint64) error {
    if daIncludedHeight == 0 {
        return nil // Nothing to finalize yet
    }
    
    // Get the block hash at the DA-included height
    finalizedHash, _, _, _, err := c.getBlockInfo(ctx, daIncludedHeight)
    if err != nil {
        return fmt.Errorf("failed to get block info for DA height %d: %w", daIncludedHeight, err)
    }
    
    c.mu.Lock()
    c.currentFinalizedBlockHash = finalizedHash
    c.mu.Unlock()
    
    return c.setFinal(ctx, finalizedHash, true)
}

Phase 3: Persist Block Hash State

// Add to EngineClient struct
type EngineClient struct {
    // ... existing fields
    store storepkg.Store // Add store reference for persistence
}

// Add persistence methods
func (c *EngineClient) persistBlockHashes() error {
    state := map[string]interface{}{
        "head": c.currentHeadBlockHash.Hex(),
        "safe": c.currentSafeBlockHash.Hex(),
        "finalized": c.currentFinalizedBlockHash.Hex(),
    }
    
    data, _ := json.Marshal(state)
    return c.store.SetMetadata(ctx, "evm_block_hashes", data)
}

func (c *EngineClient) loadBlockHashes() error {
    data, err := c.store.GetMetadata(ctx, "evm_block_hashes")
    if err != nil {
        return nil // No saved state, use defaults
    }
    
    var state map[string]interface{}
    if err := json.Unmarshal(data, &state); err != nil {
        return err
    }
    
    c.currentHeadBlockHash = common.HexToHash(state["head"].(string))
    c.currentSafeBlockHash = common.HexToHash(state["safe"].(string))
    c.currentFinalizedBlockHash = common.HexToHash(state["finalized"].(string))
    
    return nil
}

Implementation Benefits

Correct Finalization: FinalizedBlockHash reflects actual DA-included blocks
Restart Resilience: Block hash state persists across sequencer restarts
Indexer Consistency: Eliminates inconsistent finalized block hash periods
Minimal Changes: Leverages existing DA inclusion infrastructure
Backwards Compatible: Safe fallbacks prevent breaking existing deployments

Integration Points

The solution integrates with existing systems:

DA Includer: block/da_includer.go:58 already calls SetFinal() when blocks are DA-included
Store System: Uses existing metadata persistence in pkg/store/store.go
Block Manager: Coordinates with the block height tracking system

Testing Strategy

Unit Tests: Test fork choice state generation with various scenarios
Integration Tests: Verify DA height synchronization
Restart Tests: Ensure state persistence across sequencer restarts
E2E Tests: Validate indexer consistency

Would you like me to implement this solution? The changes would be focused in the EVM execution package with minimal impact on other systems.

@claude

@claude if we never sync from da an only use p2p then the finalized blockhash would never be updated

@tac0turtle

Claude finished @tac0turtle's task —— View job

EVM FinalizedHash Issue - Revised Analysis

You're absolutely right - my previous solution has a critical flaw for P2P-only nodes. Let me propose a better approach.

Todo List:

Understand the current codebase structure and find EVM-related code
Locate the problematic forkChoiceUpdatedV3 implementation
Analyze P2P block synchronization mechanism
Understand how finalization should work for P2P-only nodes
Propose revised solution handling both DA-synced and P2P-only scenarios
Provide concrete implementation details

Problem Summary

The current issue in /execution/evm/execution.go:300-304 sets all three block hashes (Head, Safe, Finalized) to prevBlockHash:

args := engine.ForkchoiceStateV1{
    HeadBlockHash:      prevBlockHash,
    SafeBlockHash:      prevBlockHash,
    FinalizedBlockHash: prevBlockHash,  // ❌ Incorrect
}

Your key insight: In P2P-only nodes (DA-only sync disabled), there's no DA inclusion mechanism, so SetFinal() would never be called, leaving the finalized hash perpetually stuck at the genesis block.

Analysis of Current Architecture

I've traced through the codebase and found:

DA-Synced Nodes: The Submitter component (block/internal/submitting/submitter.go:266) calls exec.SetFinal() when blocks are DA-included
P2P-Only Nodes: The Syncer component (block/internal/syncing/syncer.go) handles P2P block synchronization but never calls SetFinal()
EVM State: The EngineClient tracks finalized hash in memory (execution/evm/execution.go:126) but has no persistence or depth-based finalization logic

Proposed Solution: Hybrid Finalization Strategy

I propose a two-pronged approach that works for both DA-synced and P2P-only nodes:

1. For DA-Synced Nodes: Use DA Inclusion (Current Behavior)

Continue using the existing SetFinal() mechanism in the Submitter
DA inclusion provides canonical finality

2. For P2P-Only Nodes: Implement Depth-Based Finalization

Introduce a configurable finalization depth (e.g., 64 blocks, similar to Ethereum's weak subjectivity)
Automatically call SetFinal() when a block is N blocks deep
This matches the behavior expected by indexers and provides consistency

3. Persist Block Hash State Across Restarts

Save head/safe/finalized hashes to store metadata
Load on restart to maintain consistency

Implementation Plan

Phase 1: Add Depth-Based Finalization to Syncer

Location: block/internal/syncing/syncer.go

Add a finalization mechanism that tracks block depth:

// In syncer.go processLoop or after applying blocks
func (s *Syncer) maybeFinalizeByDepth(currentHeight uint64) error {
    // Only use depth-based finalization if we're not using DA
    // (DA-based finalization takes precedence)
    if s.config.DA.Address != "" {
        return nil // DA-synced nodes use DA inclusion for finality
    }
    
    // Use a safe finalization depth (e.g., 64 blocks)
    finalizationDepth := uint64(64)
    
    if currentHeight <= finalizationDepth {
        return nil // Not enough blocks yet
    }
    
    finalizedHeight := currentHeight - finalizationDepth
    
    // Check if we've already finalized this height
    lastFinalizedHeight, err := s.getLastFinalizedHeight()
    if err == nil && lastFinalizedHeight >= finalizedHeight {
        return nil // Already finalized
    }
    
    // Finalize the block at depth
    if err := s.exec.SetFinal(s.ctx, finalizedHeight); err != nil {
        return fmt.Errorf("failed to set final at depth: %w", err)
    }
    
    // Persist the finalized height
    if err := s.setLastFinalizedHeight(finalizedHeight); err != nil {
        s.logger.Warn().Err(err).Msg("failed to persist finalized height")
    }
    
    return nil
}

Phase 2: Persist EVM Block Hash State

Location: execution/evm/execution.go

Update the EngineClient to persist state:

// Add to setFinal method (around line 414)
func (c *EngineClient) setFinal(ctx context.Context, blockHash common.Hash, isFinal bool) error {
    c.mu.Lock()
    if isFinal {
        c.currentFinalizedBlockHash = blockHash
    } else {
        c.currentHeadBlockHash = blockHash
        c.currentSafeBlockHash = blockHash
    }
    
    args := engine.ForkchoiceStateV1{
        HeadBlockHash:      c.currentHeadBlockHash,
        SafeBlockHash:      c.currentSafeBlockHash,
        FinalizedBlockHash: c.currentFinalizedBlockHash,
    }
    c.mu.Unlock()
    
    // ... existing forkchoice call ...
    
    // NEW: Persist state for restart resilience
    if err := c.persistBlockHashState(ctx); err != nil {
        c.logger.Warn().Err(err).Msg("failed to persist block hash state")
        // Don't fail the operation if persistence fails
    }
    
    return nil
}

// Add persistence helper (needs access to Store)
func (c *EngineClient) persistBlockHashState(ctx context.Context) error {
    state := map[string]string{
        "head":      c.currentHeadBlockHash.Hex(),
        "safe":      c.currentSafeBlockHash.Hex(),
        "finalized": c.currentFinalizedBlockHash.Hex(),
    }
    
    data, _ := json.Marshal(state)
    // Store in a well-known metadata key
    // Note: EngineClient would need Store reference passed in constructor
    return c.store.SetMetadata(ctx, "evm_fork_choice_state", data)
}

Phase 3: Fix Startup Logic

Location: execution/evm/execution.go constructor

Load persisted state on startup:

func NewEngineExecutionClient(..., store Store) (*EngineClient, error) {
    client := &EngineClient{
        // ... existing fields ...
        store: store,
    }
    
    // Load persisted fork choice state
    if err := client.loadBlockHashState(ctx); err != nil {
        // If loading fails, use genesis as fallback
        client.currentHeadBlockHash = genesisHash
        client.currentSafeBlockHash = genesisHash
        client.currentFinalizedBlockHash = genesisHash
    }
    
    return client, nil
}

Alternative: Simpler Approach

If the above is too complex, here's a minimal fix that addresses the immediate issue:

Option A: Use a conservative finalization lag

In execution/evm/execution.go:300-304, change to:

// Calculate a safe finalized hash (lag by a fixed amount, e.g., 32 blocks)
finalizationLag := uint64(32)
var finalizedHash common.Hash

if blockHeight > finalizationLag {
    finalizedHash, _, _, _, err = c.getBlockInfo(ctx, blockHeight-1-finalizationLag)
    if err != nil {
        finalizedHash = c.genesisHash // Fallback to genesis
    }
} else {
    finalizedHash = c.genesisHash
}

args := engine.ForkchoiceStateV1{
    HeadBlockHash:      prevBlockHash,
    SafeBlockHash:      prevBlockHash,
    FinalizedBlockHash: finalizedHash,  // ✅ Use lagged finalization
}

This ensures:

✅ Works for both DA-synced and P2P-only nodes
✅ Minimal code changes
✅ Provides consistent finality to indexers
❌ But doesn't leverage DA inclusion data when available

Recommendation

I recommend implementing the full hybrid solution (Phases 1-3) because:

Correctness: DA-synced nodes get accurate finality from DA inclusion
P2P Support: P2P-only nodes get consistent depth-based finality
Restart Resilience: State persistence prevents inconsistencies after restarts
Standards Compliance: Matches Ethereum's finality semantics expected by tooling

The simpler "Option A" works but sacrifices the accuracy of DA-based finality.

Would you like me to implement the full solution or the simpler approach?

Navigation Menu

Repository navigation

EVM: Set FinalizedHash correctly in `forkChoiceUpdatedV3` call in ExecuteTxs #2402

EVM FinalizedHash Issue Analysis & Solution Proposal

Problem Analysis

Root Cause

Proposed Solution

1. Use DA Included Height for FinalizedBlockHash

2. Enhanced Implementation Plan

Implementation Benefits

Integration Points

Testing Strategy

EVM FinalizedHash Issue - Revised Analysis

Problem Summary

Analysis of Current Architecture

Proposed Solution: Hybrid Finalization Strategy

1. For DA-Synced Nodes: Use DA Inclusion (Current Behavior)

2. For P2P-Only Nodes: Implement Depth-Based Finalization

3. Persist Block Hash State Across Restarts

Implementation Plan

Phase 1: Add Depth-Based Finalization to Syncer

Phase 2: Persist EVM Block Hash State

Phase 3: Fix Startup Logic

Alternative: Simpler Approach

Recommendation

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

EVM: Set FinalizedHash correctly in forkChoiceUpdatedV3 call in ExecuteTxs #2402

Description

Activity

tac0turtle commented on Aug 8, 2025

claude commented on Aug 8, 2025

EVM FinalizedHash Issue Analysis & Solution Proposal

Problem Analysis

Root Cause

Proposed Solution

1. Use DA Included Height for FinalizedBlockHash

2. Enhanced Implementation Plan

Implementation Benefits

Integration Points

Testing Strategy

tac0turtle commented on Dec 8, 2025

claude commented on Dec 8, 2025

EVM FinalizedHash Issue - Revised Analysis

Problem Summary

Analysis of Current Architecture

Proposed Solution: Hybrid Finalization Strategy

1. For DA-Synced Nodes: Use DA Inclusion (Current Behavior)

2. For P2P-Only Nodes: Implement Depth-Based Finalization

3. Persist Block Hash State Across Restarts

Implementation Plan

Phase 1: Add Depth-Based Finalization to Syncer

Phase 2: Persist EVM Block Hash State

Phase 3: Fix Startup Logic

Alternative: Simpler Approach

Recommendation

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

EVM: Set FinalizedHash correctly in `forkChoiceUpdatedV3` call in ExecuteTxs #2402