Skip to content

fix(minibf): Unregistered dreps should figure as expired#887

Merged
scarmuega merged 2 commits intomainfrom
fix/unregistered-dreps-cant-expire
Feb 9, 2026
Merged

fix(minibf): Unregistered dreps should figure as expired#887
scarmuega merged 2 commits intomainfrom
fix/unregistered-dreps-cant-expire

Conversation

@gonzalezzfelipe
Copy link
Contributor

@gonzalezzfelipe gonzalezzfelipe commented Feb 9, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Governance now skips unnecessary expiration checks for unregistered representatives, reducing incorrect expiry decisions.
    • Improved validation for retired representatives to prevent incorrect active/expired calculations.
    • Reduced spurious state-related warnings and improved consistency of representative status handling.

@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Walkthrough

Adds DRep unregistration detection and short‑circuit guards: introduces DRepState::is_unregistered() and uses it to bypass activity/expiration calculations in loading and governance code paths for unregistered/retired DReps.

Changes

Cohort / File(s) Summary
DRepState Unregistration
crates/cardano/src/model.rs
Adds pub fn is_unregistered(&self) -> bool to determine if a DRep was unregistered (compares registered_at and unregistered_at, logs unexpected states).
Loading expiration guard
crates/cardano/src/ewrap/loading.rs
Adds early-exit in should_expire_drep() to return Ok(false) when a DRep is unregistered, skipping activity/expiration computations.
Governance model guards
crates/minibf/src/routes/governance.rs
Adds pre-checks in DrepModelBuilder methods (first_active_epoch, is_drep_expired, is_drep_retired) to short-circuit logic for unregistered/retired DReps and handle absent state safely.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • scarmuega

Poem

🐰 I sniff a DRep that hopped away,
I mark it gone, no need to stay.
Short hops, quick checks, the code runs light,
I twitch my nose and do what's right. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (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 main change: adding logic to treat unregistered DReps as expired across multiple files (model.rs, loading.rs, and governance.rs).

✏️ 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
  • Commit unit tests in branch fix/unregistered-dreps-cant-expire

No actionable comments were generated in the recent review. 🎉


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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@crates/cardano/src/model.rs`:
- Around line 1648-1654: The is_unregistered method currently uses unreachable!
in the (None, Some(_)) arm which can panic on corrupted or unexpected data;
change AccountState::is_unregistered to mirror AccountState::is_registered by
handling (None, Some(_)) safely—return false (or the appropriate boolean) and
emit a warning log instead of panicking so the process doesn't crash; update the
match in is_unregistered to remove unreachable! and add a log call (using the
existing logger or crate::log macros) referencing the unexpected (None, Some(_))
state.
🧹 Nitpick comments (1)
crates/minibf/src/routes/governance.rs (1)

127-141: is_drep_retired duplicates DRepState::is_unregistered() logic.

The match on lines 136–140 is functionally identical to DRepState::is_unregistered() added in this PR. Consider delegating to avoid the duplication.

Suggested refactor
     fn is_drep_retired(&self) -> bool {
         if self.is_special_case() {
             return false;
         }
 
         let Some(state) = self.state.as_ref() else {
             return false;
         };
 
-        match (state.registered_at, state.unregistered_at) {
-            (Some(registered), Some(unregistered)) => unregistered > registered,
-            (Some(_), None) => false,
-            _ => false,
-        }
+        state.is_unregistered()
     }

@scarmuega scarmuega merged commit 5e30af6 into main Feb 9, 2026
12 checks passed
@scarmuega scarmuega deleted the fix/unregistered-dreps-cant-expire branch February 9, 2026 21:35
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