Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 59 additions & 2 deletions crates/cardano/src/ewrap/drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use tracing::{debug, warn};

use crate::{
ewrap::{AccountId, BoundaryWork, DRepId},
AccountState, CardanoDelta, DRepState, FixedNamespace as _,
AccountState, CardanoDelta, DRepState, FixedNamespace as _, PoolDelegation,
};

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -46,6 +46,57 @@ impl dolos_core::EntityDelta for DRepExpiration {
}
}

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct PoolDelegatorRetire {
delegator: AccountId,
epoch: Epoch,

// undo
prev_pool: Option<PoolDelegation>,
}

impl PoolDelegatorRetire {
pub fn new(delegator: AccountId, epoch: Epoch) -> Self {
Self {
delegator,
epoch,
prev_pool: None,
}
}
}

impl dolos_core::EntityDelta for PoolDelegatorRetire {
type Entity = AccountState;

fn key(&self) -> NsKey {
NsKey::from((AccountState::NS, self.delegator.clone()))
}

fn apply(&mut self, entity: &mut Option<AccountState>) {
let entity = entity.as_mut().expect("account should exist");

debug!(delegator=%self.delegator, "retiring pool delegator");

// save undo info
self.prev_pool = entity.pool.live().cloned();

// apply changes
entity
.pool
.schedule(self.epoch, Some(PoolDelegation::NotDelegated));

let Some(PoolDelegation::Pool(pool)) = self.prev_pool else {
unreachable!("account delegated to pool")
};
entity.retired_pool = Some(pool);
}

fn undo(&self, _entity: &mut Option<AccountState>) {
// todo!()
// Placeholder undo logic. Ensure this does not panic.
}
Comment on lines +94 to +97
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Placeholder undo logic may cause issues during chain rollbacks.

The undo method is not implemented, which could lead to incorrect state after rollbacks. Unlike some other deltas where undo is less critical, PoolDelegatorRetire::apply modifies both entity.pool (via schedule) and entity.retired_pool. If a rollback occurs, these changes won't be reversed.

🔎 Proposed undo implementation
     fn undo(&self, _entity: &mut Option<AccountState>) {
-        // todo!()
-        // Placeholder undo logic. Ensure this does not panic.
+        let Some(entity) = _entity.as_mut() else {
+            warn!(delegator=%self.delegator, "missing account during undo");
+            return;
+        };
+
+        // Restore previous pool delegation
+        entity.pool.reset(self.prev_pool.clone());
+        // Clear the retired_pool since we're undoing the retirement
+        entity.retired_pool = None;
     }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In crates/cardano/src/ewrap/drops.rs around lines 94 to 97, implement undo to
reverse the changes made by PoolDelegatorRetire::apply: remove the scheduled
retirement entry from entity.pool.schedule that matches this delta (epoch and
pool id) and restore entity.retired_pool to its previous state (clear it or set
it back if it equals the pool id and epoch scheduled by this delta). Ensure you
safely handle None entity, only mutate when the pool and retired_pool match the
values set by apply, and avoid panics.

}

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct DRepDelegatorDrop {
delegator: AccountId,
Expand Down Expand Up @@ -107,7 +158,13 @@ impl super::BoundaryVisitor for BoundaryVisitor {
) -> Result<(), ChainError> {
let current_epoch = ctx.ending_state.number;

// NOTICE: we're not dropping delegators from retiring pools. We've been back and forth on this decision. The understanding at this point in time is that delegation remains but it just doesn't participate in the rewards.
// Notice that instead of dropping delegators when a pool is retired, we're moving the data to a different field to be able to still track the relationsihp between the pool and the delegators.

if let Some(pool) = account.delegated_pool_at(current_epoch) {
if ctx.retiring_pools.contains_key(pool) {
self.change(PoolDelegatorRetire::new(id.clone(), current_epoch));
}
}

if let Some(drep) = account.delegated_drep_at(current_epoch) {
if ctx.expiring_dreps.contains(drep) {
Expand Down
1 change: 1 addition & 0 deletions crates/cardano/src/genesis/staking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ fn parse_delegation(account: &str, pool: &str, genesis: &Genesis) -> AccountStat
registered_at: Some(0),
vote_delegated_at: None,
deregistered_at: None,
retired_pool: None,

stake: EpochValue::with_live(0, stake),
}
Expand Down
12 changes: 11 additions & 1 deletion crates/cardano/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::{
reset::{AccountTransition, EpochTransition, PoolTransition},
},
ewrap::{
drops::{DRepDelegatorDrop, DRepExpiration},
drops::{DRepDelegatorDrop, DRepExpiration, PoolDelegatorRetire},
enactment::{PParamsUpdate, TreasuryWithdrawal},
refunds::{PoolDepositRefund, ProposalDepositRefund},
rewards::AssignRewards,
Expand Down Expand Up @@ -533,6 +533,10 @@ pub struct AccountState {

#[n(6)]
pub credential: StakeCredential,

#[n(7)]
#[cbor(default)]
pub retired_pool: Option<PoolHash>,
Copy link
Member

Choose a reason for hiding this comment

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

let's put a cbor default attribute here so that previous snapshots still work regardless of the existence of the new value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved at d9b758c

}

entity_boilerplate!(AccountState, "accounts");
Expand All @@ -547,6 +551,7 @@ impl AccountState {
drep: EpochValue::new(epoch),
vote_delegated_at: None,
deregistered_at: None,
retired_pool: None,
}
}

Expand Down Expand Up @@ -1816,6 +1821,7 @@ pub enum CardanoDelta {
EpochTransition(EpochTransition),
EpochWrapUp(EpochWrapUp),
DRepDelegatorDrop(DRepDelegatorDrop),
PoolDelegatorRetire(PoolDelegatorRetire),
PoolWrapUp(PoolWrapUp),
ProposalDepositRefund(ProposalDepositRefund),
TreasuryWithdrawal(TreasuryWithdrawal),
Expand Down Expand Up @@ -1883,6 +1889,7 @@ delta_from!(PoolDepositRefund);
delta_from!(EpochTransition);
delta_from!(EpochWrapUp);
delta_from!(DRepDelegatorDrop);
delta_from!(PoolDelegatorRetire);
delta_from!(PoolWrapUp);
delta_from!(ProposalDepositRefund);
delta_from!(TreasuryWithdrawal);
Expand Down Expand Up @@ -1919,6 +1926,7 @@ impl dolos_core::EntityDelta for CardanoDelta {
Self::PoolDepositRefund(x) => x.key(),
Self::EpochTransition(x) => x.key(),
Self::EpochWrapUp(x) => x.key(),
Self::PoolDelegatorRetire(x) => x.key(),
Self::DRepDelegatorDrop(x) => x.key(),
Self::PoolWrapUp(x) => x.key(),
Self::ProposalDepositRefund(x) => x.key(),
Expand Down Expand Up @@ -1956,6 +1964,7 @@ impl dolos_core::EntityDelta for CardanoDelta {
Self::EpochTransition(x) => Self::downcast_apply(x, entity),
Self::EpochWrapUp(x) => Self::downcast_apply(x, entity),
Self::DRepDelegatorDrop(x) => Self::downcast_apply(x, entity),
Self::PoolDelegatorRetire(x) => Self::downcast_apply(x, entity),
Self::PoolWrapUp(x) => Self::downcast_apply(x, entity),
Self::ProposalDepositRefund(x) => Self::downcast_apply(x, entity),
Self::TreasuryWithdrawal(x) => Self::downcast_apply(x, entity),
Expand Down Expand Up @@ -1992,6 +2001,7 @@ impl dolos_core::EntityDelta for CardanoDelta {
Self::EpochTransition(x) => Self::downcast_undo(x, entity),
Self::EpochWrapUp(x) => Self::downcast_undo(x, entity),
Self::DRepDelegatorDrop(x) => Self::downcast_undo(x, entity),
Self::PoolDelegatorRetire(x) => Self::downcast_undo(x, entity),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find where PoolDelegatorRetire is defined
fd -t f -n "*.rs" | head -20
echo "---"
# Search for PoolDelegatorRetire definition
rg "struct PoolDelegatorRetire" --type rust -A 5
echo "---"
# Search for EntityDelta implementation for PoolDelegatorRetire
rg "impl.*EntityDelta.*PoolDelegatorRetire" --type rust -A 10

Repository: txpipe/dolos

Length of output: 1407


🏁 Script executed:

#!/bin/bash
# Check the context around line 2004 in model.rs
cat -n crates/cardano/src/model.rs | sed -n '1995,2015p'

Repository: txpipe/dolos

Length of output: 1204


🏁 Script executed:

#!/bin/bash
# Find the exact file structure for drops.rs
fd "drops.rs" --type f

Repository: txpipe/dolos

Length of output: 89


🏁 Script executed:

#!/bin/bash
# Get the full PoolDelegatorRetire implementation in drops.rs
rg "impl.*EntityDelta.*PoolDelegatorRetire" -A 50 --type rust

Repository: txpipe/dolos

Length of output: 3080


Implement the missing undo() method for PoolDelegatorRetire.

The undo() method in crates/cardano/src/ewrap/drops.rs is a placeholder (line contains // todo!()) and does not reverse the state changes made by apply(). The struct correctly captures undo information in prev_pool, but the undo() method is empty and will not restore the previous pool delegation state, breaking rollback functionality for this delta.

Self::PoolWrapUp(x) => Self::downcast_undo(x, entity),
Self::ProposalDepositRefund(x) => Self::downcast_undo(x, entity),
Self::TreasuryWithdrawal(x) => Self::downcast_undo(x, entity),
Expand Down
7 changes: 6 additions & 1 deletion crates/cardano/src/roll/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use tracing::debug;

use crate::model::FixedNamespace as _;
use crate::{model::AccountState, pallas_extras, roll::BlockVisitor};
use crate::{CardanoLogic, DRepDelegation, PParamsSet, PoolDelegation};
use crate::{CardanoLogic, DRepDelegation, PParamsSet, PoolDelegation, PoolHash};

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct TrackSeenAddresses {
Expand Down Expand Up @@ -170,6 +170,7 @@ pub struct StakeDelegation {

// undo
prev_pool: Option<PoolDelegation>,
prev_retired_pool: Option<PoolHash>,
}

impl StakeDelegation {
Expand All @@ -179,6 +180,7 @@ impl StakeDelegation {
pool,
epoch,
prev_pool: None,
prev_retired_pool: None,
}
}
}
Expand All @@ -198,17 +200,20 @@ impl dolos_core::EntityDelta for StakeDelegation {

// save undo
self.prev_pool = entity.pool.live().cloned();
self.prev_retired_pool = entity.retired_pool;

// apply changes
entity
.pool
.replace(PoolDelegation::Pool(self.pool), self.epoch);
entity.retired_pool = None;
}

fn undo(&self, entity: &mut Option<AccountState>) {
let entity = entity.as_mut().expect("existing account");

entity.pool.reset(self.prev_pool.clone());
entity.retired_pool = self.prev_retired_pool;
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/minibf/src/routes/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ impl<'a> IntoModel<AccountContent> for AccountModelBuilder<'a> {
let pool_id = self
.account_state
.delegated_pool_at(current_epoch)
.or(self.account_state.retired_pool.as_ref())
.map(bech32_pool)
.transpose()?;

Expand Down
Loading