fix: ignore error on delete in fat truncation. Case when the disk is …

[taco-paco]

…full, so delete fails

Summary

Compatibility

• No breaking changes
• Config change (describe):
• Migration needed (describe):

Testing

• tests (or explain)

Checklist

• docs updated (if needed)
• closes #

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling in ledger truncation operations; errors are now logged while processing continues instead of halting operations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[github-actions]

Manual Deploy Available

You can trigger a manual deploy of this PR branch to testnet:

Deploy to Testnet 🚀

Alternative: Comment /deploy on this PR to trigger deployment directly.

⚠️ Note: Manual deploy requires authorization. Only authorized users can trigger deployments.

Comment updated automatically when the PR is synchronized.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change modifies error handling in the truncate_slot_range function within the ledger truncator. Previously, errors from Self::delete_slots(...) were propagated early using the ? operator. The updated code catches deletion errors with if let Err(err) pattern, logs them, and continues execution to perform flush and compact operations. This alters control flow to ensure subsequent operations execute regardless of delete failures, while maintaining the same public API surface.

Suggested reviewers

• thlorenz
• bmuddha

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/ledger-truncator/delete-with-full-disk

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

magicblock-ledger/src/ledger_truncator.rs (2)

141-141: ⚠️ Potential issue | 🟡 Minor

Apply the same error-handling fix here.

Per the PR objective (ignore delete errors when disk is full), this call should also swallow and log delete errors rather than propagating them with ?.

Proposed fix

     self.ledger.set_lowest_cleanup_slot(truncate_to_slot);
-    Self::delete_slots(&self.ledger, 0, truncate_to_slot)?;
+    if let Err(err) = Self::delete_slots(&self.ledger, 0, truncate_to_slot) {
+        error!(error = ?err, "Delete failed");
+    }

---

438-444: ⚠️ Potential issue | 🟠 Major

Replace .expect() with proper error handling.

The .expect() at line 442 can panic in production if the runtime fails to build. As per coding guidelines, .unwrap() and .expect() in production Rust code should be treated as major issues requiring proper error handling or explicit justification.

Consider logging the error and returning early, or propagating the error if the start method's signature can be changed to return a Result.

Proposed fix (log and return)

         let worker_handle = thread::spawn(move || {
-            let runtime = Builder::new_current_thread()
+            let runtime = match Builder::new_current_thread()
                 .enable_all()
                 .build()
-                .expect("failed to build runtime for truncator");
-            runtime.block_on(worker.run());
+            {
+                Ok(rt) => rt,
+                Err(err) => {
+                    error!(error = ?err, "Failed to build runtime for truncator");
+                    return;
+                }
+            };
+            runtime.block_on(worker.run());
         });

🤖 Fix all issues with AI agents

In `@magicblock-ledger/src/ledger_truncator.rs`:
- Around line 284-286: The delete_slots call is handled by logging and
swallowing errors in one place but is propagated with ? inside
truncate_fat_ledger; make them consistent by changing truncate_fat_ledger to
catch the Result from Self::delete_slots(...) instead of using ?, log the error
with the same error!(error = ?err, "Delete failed") pattern, and continue, so
both call sites handle delete failures identically (refer to truncate_fat_ledger
and delete_slots to locate the change).
- Around line 283-286: The code advances the cleanup marker before attempting
deletions (ledger.set_lowest_cleanup_slot) which can leave slots marked cleaned
if Self::delete_slots fails; reorder the operations so that you first call
Self::delete_slots(ledger, from_slot, to_slot) and only on Ok(...) call
ledger.set_lowest_cleanup_slot(to_slot), or if you must keep the current order
implement error recovery by capturing the previous lowest value and restoring it
on Err(err) from Self::delete_slots; update the block around
ledger.set_lowest_cleanup_slot and Self::delete_slots accordingly and add a
short comment explaining the chosen approach.

[bmuddha]

LGTM