Skip to content
This repository was archived by the owner on Jan 16, 2026. It is now read-only.

feat: flush oracle cache on reorg #724#756

Merged
clabby merged 2 commits intoop-rs:mainfrom
piotr-roslaniec:main
Oct 30, 2024
Merged

feat: flush oracle cache on reorg #724#756
clabby merged 2 commits intoop-rs:mainfrom
piotr-roslaniec:main

Conversation

@piotr-roslaniec
Copy link
Copy Markdown
Contributor

No description provided.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.8%. Comparing base (13d7e82) to head (2b2ba67).
Report is 3 commits behind head on main.

Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Contributor

@refcell refcell left a comment

Choose a reason for hiding this comment

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

Nice work, nearly there with a few changes

Comment on lines +54 to +58

/// Flushes the cache, removing all entries.
pub fn flush(&self) {
self.cache.lock().clear();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't look like we're using this, since the FlushableCache trait provides this.

Suggested change
/// Flushes the cache, removing all entries.
pub fn flush(&self) {
self.cache.lock().clear();
}

.await?;
} else if matches!(e, ResetError::ReorgDetected(_, _)) {
self.caching_oracle.as_ref().flush();
self.pipeline.signal(Signal::FlushChannel).await?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We will need to perform a full reset on a reorg and not just a channel flush.
Basically, we need to check inside the else branch below if e matches the ResetError::ReorgDetected(_, _), then flush the caching oracle and resume with sending a reset signal.

e.g.

  } else {
      if matches!(e, ResetError::ReorgDetected(_, _)) {
            self.caching_oracle.as_ref().flush();
      }
      // Reset the pipeline to the initial L2 safe head and L1 origin,
      // and try again.
      self.pipeline
          .signal(
              ResetSignal {
                  l2_safe_head: self.l2_safe_head,
                  l1_origin: self
                      .pipeline
                      .origin()
                      .ok_or_else(|| anyhow!("Missing L1 origin"))?,
                  system_config: Some(system_config),
              }
              .signal(),
          )
          .await?;
  }

@refcell
Copy link
Copy Markdown
Contributor

refcell commented Oct 30, 2024

Great work @piotr-roslaniec!

@clabby clabby added this pull request to the merge queue Oct 30, 2024
Merged via the queue into op-rs:main with commit 712fe20 Oct 30, 2024
refcell pushed a commit that referenced this pull request Nov 1, 2024
* feat: flush oracle cache on reorg #724

* fix pr review
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Dec 10, 2025
* feat: flush oracle cache on reorg op-rs/kona#724

* fix pr review
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 14, 2026
* feat: flush oracle cache on reorg #724

* fix pr review
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants