fix: allow the ledger to rollback a state that has just been rolled forward on the volatile state

[etorreborre]

This issue has been found when trying the reconnection behavior of the node where we happen to roll forward some blocks, and then try to rollback to the last block of the volatile sequence.

The previous function was not making the distinction between finding the last point of the sequence as the rollback point and not finding it at all.

Summary by CodeRabbit

• Bug Fixes

  • Improved rollback behavior to validate target positions, handle out-of-range requests, rebuild in-memory state up to the target, and avoid incorrect error/no-op outcomes.

• Logging

  • Normalized a previously logged condition to use error-level logging.

• Tests

  • Added comprehensive rollback tests and helpers covering before-first, exact-last, middle, after-last, and between-element scenarios.

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

[coderabbitai]

Walkthrough

Refactors rollback_to in the volatile DB to validate target-slot bounds up front, restrict merging to elements with slot ≤ target_slot, break inner-loop once past the target, replace the previous post-loop error branch with a single resize_with(ix, ...) and Ok(()), adds tracing error logging tweak, and introduces rollback-focused tests and helpers.

Changes

Cohort / File(s)	Summary
Core rollback refactor crates/amaru-ledger/src/state/volatile_db.rs	Reworked rollback_to to check target_slot bounds before processing, rebuild cache up to the target, limit merging to entries with slot <= target_slot, and break the inner loop once past the target. Replaced the former post-loop error branch with resize_with(ix, ...) and Ok(()).
Logging tweak crates/amaru-ledger/src/state/volatile_db.rs	Replaced previous logging path for add.proposals.no_proposal with tracing::error! while keeping the message content.
Tests & helpers crates/amaru-ledger/src/state/volatile_db.rs (cfg(test) module)	Added a cfg(test) module that provides helpers and five rollback tests: rollback before sequence (error), rollback to last element, rollback to a middle element, rollback after sequence, and rollback between elements.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

Ah, the DB rewinds like a retro boss fight,
slots checked, loops tidy—no stray sprite,
tests queued up like quests on the board,
errors traced, cache trimmed, all accord,
rollback done neat — GG, mate. 🎮✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the core fix—enabling rollback of recently rolled-forward volatile state—which aligns perfectly with the changeset's main objective.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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

[codecov]

Codecov Report

❌ Patch coverage is 94.52055% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/amaru-ledger/src/state/volatile_db.rs	94.52%	4 Missing ⚠️

Files with missing lines	Coverage Δ
crates/amaru-ledger/src/state/volatile_db.rs	36.69% <94.52%> (+36.69%)	⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@crates/amaru-ledger/src/state/volatile_db.rs`:
- Around line 118-122: The tracing::error! call that logs rollback validation
uses the wrong word "last" in its message; update the message string in the
tracing::error! invocation (the call that includes %target_slot and first_slot =
?first.anchor.0.slot_or_default()) to replace "last known volatile state" with
"first known volatile state" so the log correctly reflects that the check is
against the first element.

🧹 Nitpick comments (1)

crates/amaru-ledger/src/state/volatile_db.rs (1)

428-536: Ripper test coverage!

These tests are like a proper walkthrough guide - covering all the key scenarios: before, after, middle, exact, and between. The test_rollback_to_exact_last_element_should_succeed test specifically nails the bug described in the PR. Clean helpers too, very DRY.

One optional consideration: you might want to add a test for rolling back on an empty VolatileDB. Currently, both boundary checks would pass through (since back() and front() return None), resulting in a no-op Ok(()). That behaviour seems reasonable, but documenting it in a test would be like leaving a note at a Zelda shrine for future devs.

💡 Optional: Test for empty VolatileDB

#[test]
fn test_rollback_on_empty_db_succeeds() {
    let mut db = VolatileDB::default();
    assert!(db.is_empty());

    let rollback_point = Point::Specific(Slot::from(10), Hash::new([0u8; 32]));
    let result = db.rollback_to(&rollback_point, |_| "Point not found");

    assert!(result.is_ok(), "Rolling back on empty DB should be a no-op");
    assert!(db.is_empty());
}