fix: Allow non-Alonzo era when missing total collateral

[caike]

This field is optional according to the CDDL.

Fixes #775

Summary by CodeRabbit

• Improvements
  • Collateral value computation now applies across a wider range of transaction eras, improving compatibility with newer and legacy transactions.
  • Relaxed strict checks let collateral be computed even when explicit collateral fields are absent, while preserving existing calculation results.
• Documentation
  • Clarified that collateral computation is era-agnostic in relevant documentation.

[coderabbitai]

📝 Walkthrough

Walkthrough

compute_collateral_value in crates/cardano/src/roll/epochs.rs had its Alonzo-only assertion removed, imports adjusted to multi-era types, and is documented to be callable for Alonzo, Babbage, or Conway transactions when total_collateral is unset; internal collateral math unchanged.

Changes

Cohort / File(s)	Summary
Collateral era relaxation crates/cardano/src/roll/epochs.rs	Removed Era from pallas::ledger::traverse import, removed debug_assert!(tx.era() == Era::Alonzo), added note that compute_collateral_value may be used for Alonzo/Babbage/Conway txs when total_collateral is absent. Core input-sum/output-subtract logic unchanged.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as Caller
  participant Compute as compute_collateral_value
  participant Tx as MultiEraTx
  Note over Compute: Accepts MultiEraTx (Alonzo/Babbage/Conway)
  Caller->>Compute: call(tx)
  Compute->>Compute: is tx.total_collateral present?
  alt present
    Compute->>Caller: return total_collateral
  else absent
    Compute->>Tx: iterate collateral inputs -> sum
    Compute->>Tx: iterate collateral-returns -> subtract outputs
    Compute->>Caller: return computed_collateral
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰
I hopped through eras, light and spry,
Removed a check with a curious eye.
Collateral counted, inputs and returns,
Across Alonzo, Babbage, Conway it learns.
Thump-thump — the ledger hums nearby.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: allowing non-Alonzo eras when the total_collateral field is missing, which directly addresses the issue.
Linked Issues check	✅ Passed	The code changes directly address issue #775 by removing the Era::Alonzo assertion and making the collateral computation era-agnostic, allowing Babbage and other eras to be handled when total_collateral is absent.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the collateral handling logic in epochs.rs; no unrelated modifications or out-of-scope work is present.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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: 0

🧹 Nitpick comments (2)

crates/cardano/src/roll/epochs.rs (2)

202-207: Consider updating the HACK comment to mention Babbage transactions.

The HACK comment (lines 202-205) mentions that "Alonzo txs don't even have the total collateral field," but based on issue #775, Babbage transactions can also have missing total_collateral fields. The new note (lines 206-207) correctly indicates the function works for multiple eras, but the HACK comment could be updated for clarity.

Apply this diff to clarify that multiple eras can have missing total_collateral:

 // HACK: There are txs that don't have an explicit value for total collateral
-// and Alonzo txs don't even have the total collateral field. This is why we
-// need to compute it by looking at collateral inputs and collateral return.
+// (Alonzo era), or the field exists but is not set (Babbage, Conway). This is
+// why we need to compute it by looking at collateral inputs and collateral return.
 // Pallas hides this from us by providing the "consumes" / "produces" facade.

---

245-245: Consider making the log message more generic.

The log message says "alonzo-style collateral computed," but this code path now executes for Babbage and Conway transactions as well when total_collateral is missing. While "alonzo-style" refers to the computation method, a more generic message would better reflect the multi-era support.

Apply this diff to make the message clearer:

-        tracing::debug!(tx=%tx.hash(), fee, "alonzo-style collateral computed");
+        tracing::debug!(tx=%tx.hash(), fee, "collateral computed from inputs/outputs");

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20973cc and 5a47282.

📒 Files selected for processing (1)

• crates/cardano/src/roll/epochs.rs (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Test (windows-latest)
• GitHub Check: Test (macos-14)
• GitHub Check: Test (macos-13)
• GitHub Check: Test (ubuntu-latest)

🔇 Additional comments (2)

crates/cardano/src/roll/epochs.rs (2)

208-232: LGTM! The collateral computation logic is correct.

The function correctly computes collateral value for invalid transactions across multiple eras by:

• Summing collateral inputs via tx.consumes()
• Subtracting collateral return via tx.produces()
• The debug_assert!(!tx.is_valid()) on line 212 appropriately ensures this is only called for invalid transactions

The removal of the era-specific assertion (mentioned in the AI summary) allows this to work for Alonzo, Babbage, and Conway transactions when total_collateral is not set, which correctly addresses issue #775.

---

8-8: Era removal is safe and will not cause compilation issues.

The only direct reference to Era in the file is at line 267, which uses the fully qualified path pallas::ledger::traverse::Era::Shelley. All other "Era" mentions are part of imported Multi-Era type names (MultiEraBlock, MultiEraTx, etc.), so the import removal will not affect them.

[scarmuega]

@caike this looks reasonable. I just need your help fixing the merge conflicts to go ahead with the merge.

[caike]

merge conflicts fixed @scarmuega