fix(cardano): stop dropping StakeVoteDeleg drep leg and drep anchors#1128
Conversation
Two Conway cert-handling gaps in delegation/drep tracking: - cert_as_vote_delegation did not match StakeVoteDeleg, so the drep leg of that cert was silently dropped and the delegator's vote delegation never recorded (the pool leg was already handled by cert_as_stake_delegation). - The anchor carried by RegDRepCert and UpdateDRepCert was discarded: DRepState had no anchor field and UpdateDRepCert only refreshed activity. DRepState now persists the anchor at a new backward- compatible CBOR index (old rows decode with None), written by a new DRepAnchorUpdate delta appended to CardanoDelta. A separate delta is used because the on-disk shapes of existing deltas are frozen for WAL compatibility. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughChangesDRep anchor support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ConwayCert
participant visit_cert
participant DRepAnchorUpdate
participant DRepState
ConwayCert->>visit_cert: provide DRep anchor
visit_cert->>DRepAnchorUpdate: emit anchor update
DRepAnchorUpdate->>DRepState: apply anchor
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/cardano/src/model/dreps.rs (1)
467-482: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a legacy CBOR decoding regression test.
The new tests cover delta undo and bincode serialization, but not loading a pre-change
DRepStaterow without field 7. Add a fixture for the legacy seven-field representation and assert thatanchordecodes asNone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/cardano/src/model/dreps.rs` around lines 467 - 482, Add a regression test near drep_anchor_update_serde_roundtrip that decodes a legacy seven-field DRepState CBOR fixture lacking field 7, then assert the resulting anchor is None. Reuse the existing CBOR decoding helpers and DRepState representation conventions, ensuring the test verifies compatibility with pre-change rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/cardano/src/model/dreps.rs`:
- Around line 467-482: Add a regression test near
drep_anchor_update_serde_roundtrip that decodes a legacy seven-field DRepState
CBOR fixture lacking field 7, then assert the resulting anchor is None. Reuse
the existing CBOR decoding helpers and DRepState representation conventions,
ensuring the test verifies compatibility with pre-change rows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 19e8112c-8502-4f80-966b-066d71a17ed1
📒 Files selected for processing (4)
crates/cardano/src/model/dreps.rscrates/cardano/src/model/mod.rscrates/cardano/src/pallas_extras.rscrates/cardano/src/roll/dreps.rs
Summary
Two Conway cert-handling gaps in delegation/DRep tracking, both on the block-roll path:
1.
StakeVoteDeleg's DRep leg was silently droppedcert_as_vote_delegation(pallas_extras.rs) matchedVoteDeleg,VoteRegDelegandStakeVoteRegDelegbut notStakeVoteDeleg, so a combined stake+vote delegation cert updated the account's pool delegation (its pool leg is handled bycert_as_stake_delegation) while the DRep delegation was never recorded. This is a live correctness bug: any account that delegated to a DRep via that cert shows no vote delegation. Fixed by adding the missing arm, emitting the existingVoteDelegationdelta.2. DRep anchors were discarded
DRepStatehad no anchor field: the anchor carried byRegDRepCertwas stashed in theDRepRegistrationdelta but never written to the entity, andUpdateDRepCertonly refreshed activity, discarding its anchor entirely.DRepStategainsanchor: Option<Anchor>at CBOR index 7 — a backward-compatible addition: pre-existing rows decode withNone, no migration.DRepAnchorUpdatedelta (appended toCardanoDelta, preserving the frozen bincode variant order) writes it, emitted for bothRegDRepCertandUpdateDRepCert. A separate delta rather than extendingDRepRegistrationbecause existing delta shapes are frozen for WAL decode compatibility (same reason as theEpochWrapUp→ V2/V3 lineage).drepAnchorwholesale (anchor := anchor), so a cert without an anchor clears the stored one. The activity/expiry refresh side ofUpdateDRepCertremains covered by the existingDRepActivitydelta.This is the first step of making Conway governance tracking real (DRep delegation at the transaction level); vote/proposal capture and ratification come separately.
Testing
drep_anchor_update_roundtrip(apply→undo reversibility) anddrep_anchor_update_serde_roundtrip(undo survives the WAL wire format);any_drep_stateextended with the anchor field.cargo clippy --workspace --all-targets --all-features: clean.cargo test --workspace --all-features: all green.cargo fmt --check: clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests