Skip to content

refactor(block): allow pruning when da disabled#3075

Open
julienrbrt wants to merge 4 commits intomainfrom
julien/pruning-2
Open

refactor(block): allow pruning when da disabled#3075
julienrbrt wants to merge 4 commits intomainfrom
julien/pruning-2

Conversation

@julienrbrt
Copy link
Member

Overview

follow-up of #2984 (ref #2984 (comment))

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedFeb 16, 2026, 12:37 PM

@claude
Copy link
Contributor

claude bot commented Feb 16, 2026

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


Code Review Complete

I've completed a comprehensive code review of this PR. The changes successfully enable block pruning for nodes without DA configured, which is an important enhancement. Here are my findings:


Overview

This PR refactors the pruning mechanism to allow nodes without Data Availability (DA) configured to prune blocks based on store height alone, while DA-enabled nodes continue to prune only DA-included blocks. The implementation is clean and well-tested.


Findings

1. CHANGELOG Entry Mismatch ⚠️

Severity: Medium | CHANGELOG.md:23

The changelog entry describes a different PR (#3073):

- Store pending blocks separately from executed blocks key. [#3073](...)

This should describe the actual changes in this PR:

- Allow pruning when the Data Availability (DA) layer is disabled. [#3075](...)

Impact: Documentation inconsistency that will confuse users reviewing the changelog.


2. Architecture & Design

Excellent design choices:

  • daEnabled flag approach (pruner.go:26,49): Clean and efficient way to conditionally handle DA-aware vs DA-agnostic pruning
  • Minimal surface area: Only passes daAddress string to determine DA enablement rather than entire DA client
  • Backward compatible: Existing DA-enabled nodes continue working identically
  • Clear separation: DA-enabled pruning waits for DA inclusion, DA-disabled pruning uses store height

The conditional logic in pruneBlocks() (pruner.go:119-131) is well-structured:

if p.daEnabled {
    // Wait for DA inclusion height
    currentDAIncluded := ...
    upperBound = min(storeHeight, currentDAIncluded)
}

3. Error Handling & Edge Cases

Well handled:

  • Missing DA height (pruner.go:124-127): Correctly skips pruning with debug log when DA is enabled but inclusion height not available yet
  • Early return on insufficient blocks (pruner.go:133-136): Prevents attempting to prune when upperBound <= KeepRecent
  • NotFound errors (pruner.go:156): Properly handles ds.ErrNotFound when exec pruner has nothing to prune

Minor observation:
The debug message at line 125 could be more informative:

p.logger.Debug().Msg("skipping pruning: DA is enabled but DA included height is not available yet")

Consider adding context about when this is expected (early in node lifecycle before first DA inclusion).


4. Test Coverage

Excellent test additions (pruner_test.go:67-165):

Two comprehensive new test cases:

  1. TestPrunerPruneBlocksWithoutDA (lines 67-126):

    • Tests empty DA address (DA disabled)
    • Verifies batch size calculation (40 blocks based on 1s interval / 100ms block time * 4)
    • Confirms blocks up to batch end are pruned
    • Validates blocks after batch are kept
    • Checks exec metadata pruning
  2. TestPrunerPruneBlocksWithDAEnabled (lines 128-165):

    • Tests non-empty DA address (DA enabled)
    • Verifies pruning is skipped when DA inclusion height unavailable
    • Confirms all blocks remain intact

Strengths:

  • Good coverage of both code paths
  • Clear test names and comments
  • Proper use of parallel tests (t.Parallel())
  • Validates both positive and negative cases

Potential enhancement:
Consider adding a test case for DA-enabled nodes with DA inclusion height set to verify they prune correctly up to the DA-included height (not just the skip case).


5. Configuration Integration

Clean integration (components.go:185,274):

Both NewSyncComponents and NewAggregatorComponents correctly pass config.DA.Address to the pruner:

pruner := pruner.New(logger, store, execPruner, config.Pruning, config.Node.BlockTime.Duration, config.DA.Address)

This is the right level of abstraction - the pruner only needs to know if DA is configured, not the entire DA client.


6. Documentation

Good documentation update (config.md:343-344):

Added clear explanation of pruning behavior differences:

- When DA is enabled (DA address is configured), pruning only removes blocks that have been confirmed on the DA layer (for mode `all`) to ensure data safety
- When DA is not enabled (no DA address configured), pruning proceeds based solely on store height, allowing nodes without DA to manage disk space

Suggestion for improvement:
The wording "for mode all" is slightly ambiguous. Consider clarifying:

- When DA is enabled (DA address is configured), pruning in `all` mode only removes blocks that have been confirmed on the DA layer to ensure data safety. Metadata-only pruning is unaffected.

7. Concurrency Safety

No new concurrency issues introduced:

  • The daEnabled field is set once during initialization (pruner.go:49) and never modified
  • Read-only access during pruning operations is safe
  • Existing mutex/synchronization patterns are preserved

8. Performance Considerations

No performance regression:

  • Additional if p.daEnabled check (pruner.go:119) is negligible (single boolean check)
  • Batch size calculation remains unchanged
  • No additional I/O operations introduced

Benefit:
DA-disabled nodes can now prune immediately based on store height without waiting for DA inclusion, potentially reducing disk usage faster.


9. Code Quality

Adheres to Go best practices:

  • Idiomatic error handling with early returns
  • Clear variable naming (daEnabled, upperBound, currentDAIncluded)
  • Appropriate use of min() function for readability
  • Consistent logging patterns with structured fields (Bool("da_enabled", p.daEnabled))

Summary

Category Rating Notes
Architecture ✅ Excellent Clean abstraction, minimal changes
Error Handling ✅ Good All edge cases covered
Testing ✅ Excellent Comprehensive new tests
Documentation ✅ Good Clear explanation of behavior
Security ✅ Good No new vulnerabilities
Performance ✅ Good No regression, potential improvement
Code Quality ✅ Excellent Idiomatic Go, well-structured

Recommendations

Must Fix

  1. Update CHANGELOG.md to describe this PR's changes instead of refactor: store pending block separately #3073

Nice to Have

  1. Add test case for DA-enabled pruning with DA inclusion height set
  2. Enhance debug message at pruner.go:125 with additional context about expected timing
  3. Clarify documentation wording about "mode all" in config.md

Conclusion

This is a well-implemented refactor that successfully enables pruning for DA-disabled nodes while maintaining backward compatibility. The code is clean, well-tested, and properly documented. The only blocking issue is the incorrect CHANGELOG entry.

Approval recommendation: Approve after CHANGELOG fix.


@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @julienrbrt, I'm Gemini Code Assist1! 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 refactors the block pruning logic to enable nodes to prune blocks regardless of whether Data Availability (DA) is configured. Previously, pruning was strictly tied to DA inclusion, which prevented non-DA nodes from managing their storage. The changes introduce a conditional pruning mechanism that adapts based on DA enablement, ensuring efficient disk space management for all node types while maintaining data integrity for DA-enabled setups.

Highlights

  • Pruning Logic Enhancement: The block pruning mechanism has been refactored to allow pruning even when Data Availability (DA) is disabled, addressing a limitation where nodes without DA configured could not manage their disk space effectively.
  • Conditional DA-Aware Pruning: A new daEnabled flag was introduced in the Pruner component, which determines whether pruning should rely on DA inclusion height. If DA is disabled, pruning proceeds based solely on the store's block height.
  • Configuration Integration: The pruner.New function now accepts the DA address from the configuration, enabling the pruner to dynamically adapt its behavior based on whether DA is active.
  • Expanded Test Coverage: New unit tests were added to thoroughly validate the pruning functionality under both DA-enabled and DA-disabled scenarios, ensuring correct behavior in various operational modes.
  • Documentation Update: The config.md documentation was updated to clearly explain the different pruning behaviors when DA is enabled versus when it is not, providing better guidance for users.

🧠 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.

Changelog
  • CHANGELOG.md
    • Updated changelog with an entry about storing pending blocks separately from executed blocks key.
  • block/components.go
    • Modified NewSyncComponents and NewAggregatorComponents to pass the DA address to the pruner initialization.
  • block/internal/pruner/pruner.go
    • Refactored the Pruner struct to include a daEnabled flag.
    • Adjusted the New function to determine daEnabled based on the provided DA address.
    • Modified the pruneBlocks method to conditionally prune based on the daEnabled flag, using DA inclusion height only when DA is active.
  • block/internal/pruner/pruner_test.go
    • Extended test suite with new cases for pruning blocks when DA is disabled.
    • Added tests to verify pruning behavior when DA is enabled and DA inclusion height is not yet available.
  • docs/learn/config.md
    • Updated documentation to explain pruning behavior with and without Data Availability (DA) enabled.
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.

@julienrbrt julienrbrt requested a review from pthmas February 16, 2026 12:35
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the pruning mechanism to allow it to function even when the Data Availability (DA) layer is disabled. This is achieved by passing the DA address to the pruner, which then conditionally checks for DA inclusion height before pruning. If DA is disabled (no address provided), pruning proceeds based on the store height. The changes are well-implemented, include thorough tests for both DA-enabled and DA-disabled scenarios, and update the documentation accordingly. My only suggestion is to correct the changelog entry to accurately reflect the changes in this PR.


### Changes

- Store pending blocks separately from executed blocks key. [#3073](https://github.com/evstack/ev-node/pull/3073)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This changelog entry appears to be from a different pull request. This PR's changes are about allowing pruning when the DA layer is disabled. Please update the changelog to reflect the correct changes.

Suggested change
- Store pending blocks separately from executed blocks key. [#3073](https://github.com/evstack/ev-node/pull/3073)
- Allow pruning when the Data Availability (DA) layer is disabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

nope, the changelog is unrelated to this pr (that do not require any)

@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.12%. Comparing base (d210055) to head (bec01ff).

Files with missing lines Patch % Lines
block/internal/pruner/pruner.go 66.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3075      +/-   ##
==========================================
+ Coverage   61.07%   61.12%   +0.04%     
==========================================
  Files         113      113              
  Lines       11431    11435       +4     
==========================================
+ Hits         6982     6990       +8     
+ Misses       3669     3657      -12     
- Partials      780      788       +8     
Flag Coverage Δ
combined 61.12% <71.42%> (+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.

@julienrbrt julienrbrt requested a review from alpe February 16, 2026 14:51
@julienrbrt julienrbrt enabled auto-merge February 16, 2026 14:51
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.

1 participant