refactor: use state da height as well

[julienrbrt]

quick follow-up of #2870

[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	Nov 20, 2025, 5:44 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 refines the initialization process for the Data Availability (DA) height within the syncer component. By incorporating the state.DAHeight into the calculation alongside the genesis and cached DA heights, the system ensures a more accurate and resilient determination of the starting DA height. This enhancement improves the overall robustness of DA synchronization, particularly in recovery situations or when previous synchronization attempts might have led to an inconsistent state.

Highlights

• DA Height Initialization: The logic for initializing the Data Availability (DA) height has been updated to consider the state.DAHeight in addition to the genesis DA height and the cached DA height. This ensures that the DA height is always set to the maximum of these three values, providing a more robust and accurate starting point for synchronization.
• Improved Robustness: The change includes a comment explaining that this modification helps in scenarios where a user might have incorrectly synced DA too far ahead. By incorporating the state.DAHeight, the system can recover more gracefully upon restart, especially if the cache is cleared, by restarting fetching from the last known block synced and executed from DA or the set genesis DA height.

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 —— View job

---

Code Review Complete

I've completed a comprehensive code review of this PR. This is a small but important refactoring that improves the robustness of DA height initialization.

---

Summary of Changes

This PR adds state.DAHeight to the DA height initialization logic in syncer.go:222. The DA height is now calculated as the maximum of three values instead of two:

• Genesis DA start height
• Cached (fetched) DA height
• State DA height (new)

Files Changed:

• block/internal/syncing/syncer.go:222 - DA height initialization logic

---

Code Review Findings

✅ Strengths

1. Improved Robustness & Recovery - The addition of state.DAHeight provides a critical safety net for recovery scenarios. If the cache is cleared (via --clear-cache), the system will now correctly use the last synced and executed DA height from state, preventing potential data loss or re-processing issues.

2. Well-Documented Intent - The inline comment clearly explains the rationale and recovery scenario, which is excellent for maintainability. The comment describes both the "what" and the "why" of this change.

3. Logical Consistency - Using max() of all three sources ensures the DA height is never set below the actual progress, which aligns with the principle of moving forward only.

4. Minimal Surface Area - The change is appropriately scoped - a single line of logic change with clear documentation. This reduces risk and makes it easy to review and test.

---

💡 Observations & Minor Suggestions

1. Edge Case Documentation (Low priority)

   The comment mentions "if a user has messed up and sync da too far ahead" - consider clarifying what "too far ahead" means in this context. Does this refer to:

   • DA height exceeding the actual available DA blocks?
   • DA height getting out of sync with executed block height?
   • Cache corruption scenarios?

   Suggestion: Consider adding a brief example or more specific description:

   // if a user has messed up and synced DA blocks ahead of execution (e.g., cache ahead of state.DAHeight),
   // on restart they can clear the cache (--clear-cache) and the retriever will restart fetching from...

2. State.DAHeight Initialization Validation (Low priority)

   The code at line 214-216 validates that state.DAHeight >= s.genesis.DAStartHeight, which is good. However, this validation happens before the max() calculation at line 222. This is correct, but consider whether additional validation might be useful:

   // After line 222, optionally log if there's a discrepancy
   if state.DAHeight != s.GetDAHeight() {
       s.logger.Debug().
           Uint64("state_da_height", state.DAHeight).
           Uint64("cache_da_height", s.cache.DaHeight()).
           Uint64("resolved_da_height", s.GetDAHeight()).
           Msg("DA height resolved from multiple sources")
   }

   This would provide visibility into recovery scenarios without changing behavior.

3. Comment Grammar (Trivial)

   Line 221: "sync da too far ahead" → "synced DA too far ahead" (past tense for consistency)

---

🔍 Architecture & Design Review

Concurrency Safety: ✅ The change maintains thread-safety. SetDAHeight() uses atomic operations, and the max() calculation happens during initialization before concurrent access begins.

State Management: ✅ The three-source approach (genesis, cache, state) creates a robust hierarchy:

• Genesis provides the floor (minimum acceptable value)
• Cache tracks what's been fetched from DA
• State tracks what's been executed and committed
• Taking the max ensures consistency and forward progress

Error Recovery: ✅ This change significantly improves error recovery. Previously, clearing the cache would lose track of fetched DA heights, potentially causing re-processing or skipped blocks. Now the state serves as a persistent record.

---

🔒 Security Review

✅ No security concerns identified:

• No new external inputs or validation requirements
• No changes to authentication, authorization, or cryptographic operations
• No potential for injection, overflow, or data leakage
• The max() operation prevents regression to lower heights, which is correct from a security standpoint

---

⚡ Performance Review

✅ No performance concerns:

• The change adds one additional value to the max() comparison (negligible cost)
• This code runs once during initialization, not in hot paths
• No new allocations or blocking operations introduced

---

🧪 Testing Considerations

Current Coverage: The change is straightforward, but consider adding test cases for:

1. Recovery Scenario Test - Verify that after clearing cache, DA height correctly uses state.DAHeight

   // Pseudo-test case
   // 1. Sync to height 100 (state.DAHeight = 100)
   // 2. Cache ahead to 150 (cache.DaHeight = 150)
   // 3. Clear cache
   // 4. Restart syncer
   // 5. Assert: DAHeight = max(genesis, 0, 100) = 100

2. Three-Way Max Test - Verify correct behavior when each source has the highest value:

   • Genesis highest: max(100, 50, 60) = 100
   • Cache highest: max(50, 100, 60) = 100
   • State highest: max(50, 60, 100) = 100

3. Regression Test - Ensure this change maintains backward compatibility with existing state and cache data

Test File Location: Tests would likely go in block/internal/syncing/syncer_test.go (if it exists) or a new integration test.

---

📚 Documentation Review

✅ Inline Documentation: Excellent - the comment clearly explains the logic and recovery scenario.

⚠️ Potential Gaps:

1. If there's user-facing documentation about the --clear-cache flag, ensure it references this recovery behavior
2. Consider documenting this in any operational runbooks or troubleshooting guides

---

Overall Assessment

Verdict: ✅ Approve - High Quality Change

This is a well-thought-out refactoring that:

• Improves system robustness and recovery capabilities
• Is clearly documented with helpful inline comments
• Maintains all safety invariants (thread-safety, validation, forward-progress)
• Has minimal risk due to small scope
• Logically follows from PR refactor: retrieve highest da height in cache #2870's work on cache DA height

The change is production-ready. The minor suggestions above are optional enhancements that could be addressed in follow-up work if desired.

Recommended Actions:

1. ✅ Merge as-is (the change is solid)
2. 📝 Optional: Consider the test cases mentioned above
3. 📝 Optional: Minor comment clarity improvements

---

Related Context

This PR builds on #2870 which introduced cache.DaHeight(). Together, these changes create a comprehensive three-source DA height tracking system that's more resilient to cache clearing and restart scenarios.

[gemini-code-assist]

Code Review

This pull request correctly enhances the DA height initialization logic by incorporating the DA height from the persisted state. This change improves the syncer's robustness, especially during restarts. The implementation is sound. I have provided one suggestion to improve the clarity and conciseness of the accompanying code comment.

[gemini-code-assist]

The existing comment is a bit informal and verbose. A more concise and professional comment would improve code clarity and maintainability.

Suggested change

	// we get the max from the genesis da height, the state da height and the cache (fetched) da height
	// if a user has messed up and sync da too far ahead, on restart they can clear the cache (--clear-cache) and the retrieve will restart fetching from the last known block synced and executed from DA or the set genesis da height.
	// Set DA height to the maximum of the genesis start height, the state's DA height, and the cached DA height.
	// This ensures we resume from the highest known DA height, even if the cache is cleared on restart.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.94%. Comparing base (faabb32) to head (665854e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2872   +/-   ##
=======================================
  Coverage   64.94%   64.94%           
=======================================
  Files          81       81           
  Lines        7262     7262           
=======================================
  Hits         4716     4716           
  Misses       2001     2001           
  Partials      545      545           

Flag	Coverage Δ
combined	64.94% <100.00%> (ø)

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.