fix(sync): apply deltas before WAL commit so rollback undo works

[scarmuega]

Summary

The roll work-unit lifecycle wrote the WAL before apply_entities ran, so every on-disk delta carried prev_*: None. When core/sync.rs::rollback later deserialized those rows and called delta.undo(), the first non-trivial delta (typically ControlledAmountInc) panicked with:

panicked at crates/cardano/src/model/accounts.rs:329:47: apply captured stake

This affected anyone who bootstrapped from a snapshot, started syncing, and then hit any peer-driven rollback past blocks dolos had already roll-forwarded since startup.

The ordering itself is pre-existing (set when work units were formalized in #842), but the panic was latent until #971 replaced no-op undo stubs with real expect("apply captured ...") calls. The proptest harness in #971 only round-trips deltas in memory, so it never exercised the WAL serde path.

What changed

• RollWorkUnit lifecycle reshuffle (crates/cardano/src/roll/work_unit.rs): load_entities moved into load, apply_entities moved into compute. commit_wal now serializes deltas that already carry prev_* undo state. commit_state becomes thin (just persist).
• Invariant guard (crates/cardano/src/roll/batch.rs): new applied: bool flag on WorkBatch, set by apply_entities, asserted by commit_wal via debug_assert!. A future re-ordering trips locally rather than in production rollback.
• Reverse undo order (crates/core/src/sync.rs): the inner loop in rollback iterated deltas forward when undoing. Multiple deltas keyed to the same entity must be reversed last-first to walk back through the apply chain correctly. Was masked while prev_* was None; load-bearing once it's populated.
• New proptest helper (crates/cardano/src/model/testing.rs): assert_delta_serde_roundtrip serializes the post-apply delta with bincode (the WAL's encoding), deserializes, and asserts undo restores the original entity.
• Serde-roundtrip proptests for ControlledAmountInc, ControlledAmountDec, StakeRegistration, StakeDelegation, StakeDeregistration, VoteDelegation, WithdrawalInc.
• Integration test (tests/bootstrap.rs::test_rollback_after_full_sync_lifecycle): feeds blocks through the full sync lifecycle and rolls back. On the un-fixed code it dies with the exact panic from the bug report; with the fix it passes cleanly.

Verification

• The integration test reproduces the user's exact panic when run against the un-fixed lifecycle (crates/cardano/src/model/accounts.rs:329:47: apply captured stake).
• With the fix, full workspace test suite passes — 0 failures across all crates plus integration tests, including 117 cardano lib tests.

Out of scope (flagged for follow-up)

• Boundary work units (Ewrap, Estart, Rupd) don't write deltas to the WAL today (they inherit the default no-op commit_wal), so rollbacks across an epoch boundary still don't undo the boundary deltas. Separate gap.
• State catch-up from WAL after a crash between commit_wal and commit_state — same recovery window as before this fix; current recovery relies on the peer re-sending the block.

Test plan

• cargo test --test bootstrap — passes (3/3, including the new regression)
• cargo test -p dolos-cardano --lib — passes (117/117, including the new serde-roundtrip proptests)
• cargo test --workspace — passes, 0 failures
• Confirm the new integration test fails with the exact production panic when applied against an un-fixed RollWorkUnit
• Smoke-test on a live snapshot bootstrap + sync (recommended before merge — the unit/integration coverage validates the lifecycle but exercising a real chainsync rollback against a peer is the final gate)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved rollback consistency by ensuring deltas are undone in the correct order during state recovery.

• Tests

  • Added comprehensive serialization roundtrip validation tests.
  • Added integration test for rollback recovery after full sync lifecycle.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The pull request reorganizes the roll pipeline to apply entity deltas earlier during computation rather than at commit time, ensuring undo (prev_*) state is captured before WAL serialization. Comprehensive serde roundtrip tests validate delta undo behavior through bincode serialization, and rollback undo logic now traverses deltas in reverse order for correctness. An integration test verifies full rollback lifecycle.

Changes

Cohort / File(s)	Summary
Dependencies crates/cardano/Cargo.toml	Added bincode as workspace-managed development dependency for serde roundtrip testing.
Test Infrastructure & Helpers crates/cardano/src/model/testing.rs, crates/cardano/src/model/accounts.rs, tests/bootstrap.rs	Introduced assert_delta_serde_roundtrip helper for validating WAL-style serialization/deserialization of deltas; added seven property tests in accounts module exercising undo after bincode roundtrips; added integration test for full rollback lifecycle after block sync.
Roll Pipeline Reorganization crates/cardano/src/roll/work_unit.rs, crates/cardano/src/roll/batch.rs	Shifted entity loading to load() phase and delta application to compute() phase; commit_wal() no longer performs slot sorting or entity application; added applied flag tracking in WorkBatch with assertion ensuring deltas are applied before WAL commit; comments updated to reflect control flow.
Rollback Undo Ordering crates/core/src/sync.rs	Modified rollback to traverse log.delta in reverse order when undoing, ensuring last-applied-first-undone semantics; added inline comments describing ordering requirement tied to each delta's pre-apply captured state.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• fix: catch-up stores during init #947: Modifies the same roll/batch.rs work-batch commit flow and adjusts bootstrap test patterns.
• refactor(cardano): avoid excessive data in account entity #672: Refactors AccountState fields and delta apply/undo implementations alongside WAL serialization behavior.
• fix(cardano): use entity streaming for mem-heavy work units #846: Adjusts commit/work-unit pipeline to change when entity deltas are applied versus persisted.

Poem

🐰 The deltas now apply before they dance with WAL,
In reverse we undo them, heeding logic's call,
Bincode tests the round-trip through serialize's hall,
Prev-state captured early, undo restores all!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: reordering the WAL commit lifecycle so deltas carry undo state, enabling correct rollback behavior.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/wal-apply-order-rollback-undo

---

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

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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