Skip to content

Conversation

@Kourin1996
Copy link

Closes https://github.com/celo-org/celo-blockchain-planning/issues/940

This PR adds code to retrieve and initialize L1 Safe and Finalized head in SyncStatus when OpNode starts, preventing Sequencer from failing to fetch the next L1 finalized block if --sequencer.use-finalized is enabled.

@Kourin1996 Kourin1996 self-assigned this Mar 19, 2025
@palango palango requested review from ezdac, karlb, palango and seolaoh March 19, 2025 14:38
@Kourin1996 Kourin1996 marked this pull request as ready for review March 19, 2025 15:24
log.Info("Rollup node started")

// If n.cfg.Driver.SequencerUseFinalized is true, sequencer cannot retrieve non-finalized L1 blocks.
// OpNode periodically fetches the latest safe and finalized L1 blocks every epoch (13 minutes),
Copy link

Choose a reason for hiding this comment

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

That should be 6.4 minutes I think. An epoch is 32*12 seconds, which is the default for the L1EpochPollIntervalFlag

@palango
Copy link

palango commented Mar 19, 2025

Do we have understanding what blocks the node from fixing itself, i.e. getting an up-to-date L1 finalized reference when the node ran out of the seq drift window?

@Kourin1996
Copy link
Author

Do we have understanding what blocks the node from fixing itself, i.e. getting an up-to-date L1 finalized reference when the node ran out of the seq drift window?

@palango As far as I see the code, there's no way to recover the sequencing except for admin API.

FYR, someone raised a similar issue.
ethereum-optimism/developers#701

@seolaoh
Copy link

seolaoh commented Mar 20, 2025

I'm wondering if it's better to consolidate the L1 block fetching logic in one place. Specifically, modifying this code block so that we call fetchAndSignal function immediately before waiting for the first ticker signal.

		fetchAndSignal := func() {
			reqCtx, reqCancel := context.WithTimeout(eventsCtx, timeout)
			ref, err := src.L1BlockRefByLabel(reqCtx, label)
			reqCancel()
			if err != nil {
				log.Warn("failed to poll L1 block", "label", label, "err", err)
			} else {
				fn(eventsCtx, ref)
			}
		}
		fetchAndSignal()

		ticker := time.NewTicker(interval)
		defer ticker.Stop()
		for {
			select {
			case <-ticker.C:
				fetchAndSignal()
			case <-eventsCtx.Done():
				return nil
			}
		}

@Kourin1996
Copy link
Author

Kourin1996 commented Mar 20, 2025

I'm wondering if it's better to consolidate the L1 block fetching logic in one place. Specifically, modifying this code block so that we call fetchAndSignal function immediately before waiting for the first ticker signal.

@seolaoh Generally speaking, I think it will be better in terms of cleanliness, too. However, the value is set only after l2Driver is initialized (inside initL2), which happens after polling is set up (inside initL1). I could add some delays in the polling function, but I think the original approach ensures that the finalized head is fetched and set properly.

func (n *OpNode) OnNewL1Finalized(ctx context.Context, sig eth.L1BlockRef) {
	if n.l2Driver == nil {
		return
	}
	// Pass on the event to the L2 Engine
	ctx, cancel := context.WithTimeout(ctx, time.Second*10)
	defer cancel()
	if err := n.l2Driver.OnL1Finalized(ctx, sig); err != nil {
		n.log.Warn("failed to notify engine driver of L1 finalized block change", "err", err)
	}
}

@seolaoh
Copy link

seolaoh commented Mar 20, 2025

I'm wondering if it's better to consolidate the L1 block fetching logic in one place. Specifically, modifying this code block so that we call fetchAndSignal function immediately before waiting for the first ticker signal.

@seolaoh Generally speaking, I think it will be better in terms of cleanliness, too. However, the value is set only after l2Driver is initialized (inside initL2), which happens after polling is set up (inside initL1). I could add some delays in the polling function, but I think the original approach ensures that the finalized head is fetched and set properly.

func (n *OpNode) OnNewL1Finalized(ctx context.Context, sig eth.L1BlockRef) {
	if n.l2Driver == nil {
		return
	}
	// Pass on the event to the L2 Engine
	ctx, cancel := context.WithTimeout(ctx, time.Second*10)
	defer cancel()
	if err := n.l2Driver.OnL1Finalized(ctx, sig); err != nil {
		n.log.Warn("failed to notify engine driver of L1 finalized block change", "err", err)
	}
}

Oh I didn't know that, then the original approach seems solid to set the finalized head.

Copy link

@piersy piersy left a comment

Choose a reason for hiding this comment

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

Hi @Kourin1996. At present this looks like the best we can do without significant further investigation. But I would say that this does not close the linked issue, since the underlying problem (that the node stalls permanently when maxSequencerDrift is exceeded) remains. So lets merge this but keep the referenced ticket open.

@Kourin1996 Kourin1996 deleted the Kourin1996/fix-finalized-flag-issue branch March 21, 2025 10:59
karlb pushed a commit that referenced this pull request Mar 24, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
piersy pushed a commit that referenced this pull request May 7, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
piersy pushed a commit that referenced this pull request May 8, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
piersy pushed a commit that referenced this pull request May 8, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
piersy pushed a commit that referenced this pull request May 8, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
piersy pushed a commit that referenced this pull request May 20, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
piersy pushed a commit that referenced this pull request May 22, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 23, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 25, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 26, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 26, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 27, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 27, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 28, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 28, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Jul 28, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 4, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 4, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 6, 2025
…#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 7, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 8, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 8, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 11, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
Kourin1996 added a commit that referenced this pull request Aug 11, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
gastonponti pushed a commit that referenced this pull request Sep 22, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
gastonponti pushed a commit that referenced this pull request Sep 22, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
gastonponti pushed a commit that referenced this pull request Sep 23, 2025
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
gastonponti pushed a commit that referenced this pull request Jan 8, 2026
…ode startup (#367)

* Set L1 safe and finalized head at startup of op-node

* Wrap initialization of L1 safe & finalized head in SyncStatus with if block

* Fix comment

* Fix commen

* Fix comment

* Fix codes based on feedback

* Fix comment

* Swap the order of fetching finalized and safe L1 block references

* Move L1 safe and finalized head fetching to the beginning of OpNode::Start

* Remove unnecessary empty line

* Add log in finalized
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.

5 participants