Skip to content

Conversation

@fridrik01
Copy link
Contributor

@fridrik01 fridrik01 commented Nov 24, 2025

This PR fixes potential orphaned blob sidecars on disk from incomplete block finalization.

If FinalizeBlock fails after FinalizeSidecars has persisted blob sidecars to disk, we may leave behind orphaned blobs at slot lastBlockHeight + 1. These orphaned blobs:

  • Consume disk space unnecessarily
  • Represent incomplete/invalid state since no corresponding block was finalized
  • Persist across node restarts

This PR addresses this so that on node startup (during CometBFT service initialization), check for and remove any orphaned blob sidecars at slot lastBlockHeight + 1.

@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

❌ Patch coverage is 69.44444% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.26%. Comparing base (e293ced) to head (58aaca5).

Files with missing lines Patch % Lines
beacon/blockchain/service.go 73.91% 4 Missing and 2 partials ⚠️
storage/filedb/range_db.go 66.66% 2 Missing and 1 partial ⚠️
consensus/cometbft/service/service.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2988      +/-   ##
==========================================
+ Coverage   63.10%   63.26%   +0.15%     
==========================================
  Files         353      353              
  Lines       17049    17085      +36     
==========================================
+ Hits        10759    10809      +50     
+ Misses       5447     5424      -23     
- Partials      843      852       +9     
Files with missing lines Coverage Δ
da/store/store.go 64.00% <100.00%> (+16.08%) ⬆️
consensus/cometbft/service/service.go 42.35% <0.00%> (+0.68%) ⬆️
storage/filedb/range_db.go 77.22% <66.66%> (+16.35%) ⬆️
beacon/blockchain/service.go 80.00% <73.91%> (-4.38%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fridrik01 fridrik01 marked this pull request as ready for review November 24, 2025 15:19
@fridrik01 fridrik01 requested a review from a team as a code owner November 24, 2025 15:19
Copilot AI review requested due to automatic review settings November 24, 2025 15:19
Copilot finished reviewing on behalf of fridrik01 November 24, 2025 15:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses the issue of orphaned blob sidecars that can remain on disk when block finalization fails after blob sidecars have been persisted. The solution involves detecting and cleaning up these orphaned blobs during node startup.

Key Changes:

  • Implements PruneOrphanedBlobs method in blockchain service to detect and remove orphaned blob sidecars at lastBlockHeight + 1
  • Adds DeleteByIndex method to the storage layer that removes entries without modifying the lower bound index (unlike Prune)
  • Integrates orphaned blob cleanup into CometBFT service initialization to run automatically on node startup

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
beacon/blockchain/service.go Implements PruneOrphanedBlobs method with orphaned blob detection and cleanup logic
beacon/blockchain/interfaces.go Adds PruneOrphanedBlobs method to BlockchainI interface
consensus/cometbft/service/service.go Calls PruneOrphanedBlobs during service initialization after loading the last block height
da/store/store.go Implements DeleteBlobSidecars method that delegates to IndexDB.DeleteByIndex
da/store/interfaces.go Adds DeleteByIndex method to IndexDB interface
storage/filedb/range_db.go Implements DeleteByIndex method that removes entries at a specific index without updating lowerBoundIndex
storage/filedb/range_db_test.go Adds comprehensive test verifying DeleteByIndex doesn't affect the lower bound index invariant
testing/simulated/testnode.go Exposes Blockchain service in TestNode for test access
testing/simulated/orphaned_blobs_test.go Adds integration test simulating orphaned blob creation and cleanup on node restart

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants