chore(sequencers): optimize store check

[julienrbrt]

Overview

Small improvement to check da inclusion store once.

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Dec 19, 2025, 9:16 PM

[gemini-code-assist]

Summary of Changes

Hello @julienrbrt, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on optimizing the retrieval of the Data Availability (DA) start height within the sequencer. By implementing an in-memory cache, the system can now avoid unnecessary database lookups for this frequently accessed value, leading to improved performance and more efficient state management. The change also includes a minor refactoring to better integrate the retrieval logic with the Sequencer struct.

Highlights

• Performance Optimization: Introduced an in-memory caching mechanism for the daStartHeight within the Sequencer to reduce redundant database reads.
• Code Refactoring: The getInitialDAStartHeight function has been refactored from a package-level function into a method of the Sequencer struct, improving encapsulation.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[claude]

Claude finished @julienrbrt's task in 1m 19s —— View job

---

Code Review Complete

I've analyzed the optimization in this PR. Here's my comprehensive review:

---

Summary

This PR introduces an in-memory cache for daStartHeight to avoid redundant database lookups. The implementation is sound and provides a measurable performance improvement for frequently-called code paths.

---

Positive Observations

1. Performance Optimization ✓

• The caching mechanism effectively eliminates redundant database reads of the genesis DA height
• The atomic.Uint64 field provides thread-safe access without mutex overhead
• Smart lazy-loading pattern: only queries the store if cache is uninitialized (value is 0)

2. Code Quality ✓

• Clean refactoring from package-level function to method improves encapsulation
• Proper use of atomic.Uint64 for lock-free concurrent access
• Maintains existing error handling patterns

3. Correctness ✓

• Initialization in NewSequencer ensures cache is populated from genesis config
• Store lookup happens only when needed (lazy initialization on first access)
• Cache-aside pattern is correctly implemented with atomic operations

---

Areas for Consideration

1. Cache Invalidation Strategy

The current implementation assumes daStartHeight never changes after initialization. While this is likely valid for genesis DA height, there's a subtle edge case:

// In getInitialDAStartHeight
if daStartHeight := c.daStartHeight.Load(); daStartHeight != 0 {
    return daStartHeight
}

Issue: If the cached value is 0, the code will always query the store, even after storing 0 from the database. This creates a scenario where:

• If store.GenesisDAHeightKey returns an error or invalid data → returns 0
• Next call with cache value 0 → queries store again (redundant)
• This defeats the optimization for the 0 case

Recommendation: Consider using a separate boolean flag or a pointer to distinguish between "not initialized" and "initialized to 0":

type Sequencer struct {
    // ...
    daStartHeight        atomic.Uint64
    daStartHeightCached  atomic.Bool  // Track if value has been loaded
}

func (c *Sequencer) getInitialDAStartHeight(ctx context.Context) uint64 {
    if c.daStartHeightCached.Load() {
        return c.daStartHeight.Load()
    }
    
    s := store.New(store.NewEvNodeKVStore(c.db))
    daIncludedHeightBytes, err := s.GetMetadata(ctx, store.GenesisDAHeightKey)
    
    var daStartHeight uint64
    if err == nil && len(daIncludedHeightBytes) == 8 {
        daStartHeight = binary.LittleEndian.Uint64(daIncludedHeightBytes)
    }
    
    c.daStartHeight.Store(daStartHeight)
    c.daStartHeightCached.Store(true)
    
    return daStartHeight
}

However, if daStartHeight == 0 is truly an invalid/edge case in your system, the current implementation is acceptable.

2. Race Condition (Minor)

There's a theoretical TOCTOU (time-of-check-time-of-use) race in getInitialDAStartHeight:

if daStartHeight := c.daStartHeight.Load(); daStartHeight != 0 {
    return daStartHeight  // Fast path
}

// Slow path: multiple goroutines could execute this simultaneously
s := store.New(store.NewEvNodeKVStore(c.db))
daIncludedHeightBytes, err := s.GetMetadata(ctx, store.GenesisDAHeightKey)
// ...
c.daStartHeight.Store(daStartHeight)

Impact: If multiple goroutines call getInitialDAStartHeight concurrently before the cache is populated, they'll all query the database simultaneously. This is harmless (idempotent reads with eventual consistency) but defeats the optimization temporarily.

Recommendation: If this is a hot path during initialization, consider using sync.Once for one-time initialization:

type Sequencer struct {
    // ...
    daStartHeight     atomic.Uint64
    daStartHeightOnce sync.Once
}

func (c *Sequencer) getInitialDAStartHeight(ctx context.Context) uint64 {
    c.daStartHeightOnce.Do(func() {
        if daStartHeight := c.daStartHeight.Load(); daStartHeight != 0 {
            return // Already initialized from genesis
        }
        
        s := store.New(store.NewEvNodeKVStore(c.db))
        daIncludedHeightBytes, err := s.GetMetadata(ctx, store.GenesisDAHeightKey)
        if err == nil && len(daIncludedHeightBytes) == 8 {
            c.daStartHeight.Store(binary.LittleEndian.Uint64(daIncludedHeightBytes))
        }
    })
    
    return c.daStartHeight.Load()
}

Note: This requires careful handling since sync.Once.Do doesn't accept a context, but the database query needs one.

3. Documentation

Consider adding a comment to the daStartHeight field explaining its caching behavior:

// daStartHeight caches the DA height from genesis to avoid repeated store lookups.
// Initialized from genesis.DAStartHeight and updated from store on first access if needed.
daStartHeight atomic.Uint64

---

Testing Recommendations

1. Unit test for cache behavior: Verify that getInitialDAStartHeight only queries the store once
2. Edge case for zero value: Test behavior when store returns valid 0 vs. error
3. Concurrent access test: Ensure thread-safety under concurrent calls

---

Verdict

Approved with minor suggestions. The optimization is effective and the implementation is correct for the common case. The suggestions above are for edge case handling and could be addressed in a follow-up if deemed necessary.

The code follows Go idioms, uses appropriate concurrency primitives, and provides a measurable performance improvement without introducing correctness issues.

Impact: Reduced database I/O, improved performance for forced inclusion transaction retrieval paths.

---

[gemini-code-assist]

Code Review

This pull request introduces an optimization to cache the initial DA start height, avoiding repeated lookups from the store. A new atomic field daStartHeight is added to the Sequencer struct, and the getInitialDAStartHeight function is converted to a method that implements read-through caching. The change is logical and addresses the goal of checking the store only once. However, the current implementation of the caching logic in getInitialDAStartHeight has a race condition that could lead to multiple database reads under concurrent access. I've left a comment with a suggestion to address this.

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.15%. Comparing base (d386df5) to head (260613e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
sequencers/single/sequencer.go	50.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2945      +/-   ##
==========================================
- Coverage   59.18%   59.15%   -0.04%     
==========================================
  Files          90       90              
  Lines        8627     8632       +5     
==========================================
  Hits         5106     5106              
- Misses       2940     2944       +4     
- Partials      581      582       +1     

Flag	Coverage Δ
combined	59.15% <50.00%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[alpe]

LGTM 👍