From c16c3ff6af5cae6dbd0107cf3b1df17d4000e84a Mon Sep 17 00:00:00 2001 From: Paul Clark Date: Tue, 28 Oct 2025 17:45:11 +0000 Subject: [PATCH 1/3] Force networkId on pool reward accounts One early SPO used a testnet address (e0...) as its reward account, and registered the same hash on mainnet (e1...). It still got its reward in epoch 211 so this must have been accepted! --- codec/src/map_parameters.rs | 8 +++++++- common/src/address.rs | 2 +- modules/accounts_state/src/rewards.rs | 11 +++++++++-- modules/accounts_state/src/snapshot.rs | 7 +++++-- modules/accounts_state/src/state.rs | 12 ++++++++++-- modules/spo_state/src/spo_state.rs | 2 +- modules/spo_state/src/state.rs | 2 ++ 7 files changed, 35 insertions(+), 9 deletions(-) diff --git a/codec/src/map_parameters.rs b/codec/src/map_parameters.rs index 05064345..d79a6959 100644 --- a/codec/src/map_parameters.rs +++ b/codec/src/map_parameters.rs @@ -376,7 +376,13 @@ pub fn map_certificate( numerator: margin.numerator, denominator: margin.denominator, }, - reward_account: StakeAddress::from_binary(reward_account)?, + // Force networkId - in mainnet epoch 208, one SPO (c63dab6d780a) uses + // an e0 (testnet!) address, and this then fails to match their actual + // reward account (e1). Feels like this should have been + // a validation failure, but clearly wasn't! + reward_account: StakeAddress::new( + StakeAddress::from_binary(reward_account)?.payload, + network_id.clone()), pool_owners: pool_owners .into_iter() .map(|v| { diff --git a/common/src/address.rs b/common/src/address.rs index 7c5e6b3a..46cee671 100644 --- a/common/src/address.rs +++ b/common/src/address.rs @@ -484,7 +484,7 @@ impl StakeAddress { impl Display for StakeAddress { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", hex::encode(self.get_credential().get_hash())) + write!(f, "{}", hex::encode(self.to_binary())) } } diff --git a/modules/accounts_state/src/rewards.rs b/modules/accounts_state/src/rewards.rs index 7632ba54..1bf4cf0a 100644 --- a/modules/accounts_state/src/rewards.rs +++ b/modules/accounts_state/src/rewards.rs @@ -52,6 +52,7 @@ pub struct RewardsResult { /// Calculate rewards for a given epoch based on current rewards state and protocol parameters /// The epoch is the one that has just ended - we assume the snapshot for this has already been /// taken. +/// Registrations/deregistrations are net changes between 'staking' and 'performance' snapshots /// Note immutable - only state change allowed is to push a new snapshot pub fn calculate_rewards( epoch: u64, @@ -130,10 +131,16 @@ pub fn calculate_rewards( // Also, to handle the early Shelley timing bug, we allow it if it was registered // during the current epoch if !pay_to_pool_reward_account { - debug!("Checking old reward account {}", staking_spo.reward_account); + debug!("Checking old reward account {} for late registration", + staking_spo.reward_account); // Note we use the staking reward account - it could have changed pay_to_pool_reward_account = registrations.contains(&staking_spo.reward_account); + + if pay_to_pool_reward_account { + info!("SPO {}'s reward account {} was registered in this epoch", + hex::encode(operator_id), staking_spo.reward_account); + } } // There was a bug in the original node from Shelley until Allegra where if multiple SPOs @@ -161,7 +168,7 @@ pub fn calculate_rewards( } } else { info!( - "Reward account for SPO {} was deregistered", + "Reward account for SPO {} isn't registered", hex::encode(operator_id) ) } diff --git a/modules/accounts_state/src/snapshot.rs b/modules/accounts_state/src/snapshot.rs index e1aab49c..8b2e347c 100644 --- a/modules/accounts_state/src/snapshot.rs +++ b/modules/accounts_state/src/snapshot.rs @@ -60,7 +60,7 @@ pub struct Snapshot { } impl Snapshot { - /// Get a stake snapshot based the current stake addresses + /// Get a stake snapshot based on the current stake addresses #[allow(clippy::too_many_arguments)] pub fn new( epoch: u64, @@ -95,6 +95,9 @@ impl Snapshot { .unwrap_or(false), None => false, }; + debug!(epoch, previous_epoch=two_previous_snapshot.epoch, + "Two previous reward account for SPO {} registered: {}", + hex::encode(spo_id), two_previous_reward_account_is_registered); // Add the new one snapshot.spos.insert( @@ -128,7 +131,7 @@ impl Snapshot { // SPO has retired - this stake is simply ignored debug!( epoch, - "SPO {} for hash {} retired? Ignored", + "SPO {} for stake address {} retired? Ignored", hex::encode(spo_id), stake_address ); diff --git a/modules/accounts_state/src/state.rs b/modules/accounts_state/src/state.rs index 267c51b8..3395bea4 100644 --- a/modules/accounts_state/src/state.rs +++ b/modules/accounts_state/src/state.rs @@ -385,8 +385,8 @@ impl State { ); if tracing::enabled!(Level::DEBUG) { - registrations.iter().for_each(|addr| debug!("Registration {}", addr)); - deregistrations.iter().for_each(|addr| debug!("Deregistration {}", addr)); + registrations.iter().for_each(|addr| debug!(epoch, "Registration {}", addr)); + deregistrations.iter().for_each(|addr| debug!(epoch, "Deregistration {}", addr)); } // Calculate reward payouts for previous epoch @@ -712,6 +712,7 @@ impl State { /// Handle an SPOStateMessage with the full set of SPOs valid at the end of the last /// epoch pub fn handle_spo_state(&mut self, spo_msg: &SPOStateMessage) -> Result<()> { + // Capture current SPOs, mapped by operator ID let new_spos: OrdMap = spo_msg.spos.iter().cloned().map(|spo| (spo.operator.clone(), spo)).collect(); @@ -734,12 +735,14 @@ impl State { if spo.pledge != old_spo.pledge || spo.cost != old_spo.cost || spo.margin != old_spo.margin + || spo.reward_account != old_spo.reward_account { debug!( epoch = spo_msg.epoch, pledge = spo.pledge, cost = spo.cost, margin = ?spo.margin, + reward = %spo.reward_account, "Updated parameters for SPO {}", hex::encode(id) ); @@ -752,6 +755,7 @@ impl State { pledge = spo.pledge, cost = spo.cost, margin = ?spo.margin, + reward = %spo.reward_account, "Registered new SPO {}", hex::encode(id) ); @@ -791,6 +795,8 @@ impl State { /// Register a stake address, with a specified deposit if known fn register_stake_address(&mut self, stake_address: &StakeAddress, deposit: Option) { + + debug!("Register stake address {stake_address}"); // Stake addresses can be registered after being used in UTXOs let mut stake_addresses = self.stake_addresses.lock().unwrap(); if stake_addresses.register_stake_address(stake_address) { @@ -819,6 +825,8 @@ impl State { /// Deregister a stake address, with specified refund if known fn deregister_stake_address(&mut self, stake_address: &StakeAddress, refund: Option) { + debug!("Deregister stake address {stake_address}"); + // Check if it existed let mut stake_addresses = self.stake_addresses.lock().unwrap(); if stake_addresses.deregister_stake_address(stake_address) { diff --git a/modules/spo_state/src/spo_state.rs b/modules/spo_state/src/spo_state.rs index c44628e4..a3570165 100644 --- a/modules/spo_state/src/spo_state.rs +++ b/modules/spo_state/src/spo_state.rs @@ -272,7 +272,7 @@ impl SPOState { } } - // Handle EochActivityMessage + // Handle EpochActivityMessage let (_, ea_message) = ea_message_f.await?; if let Message::Cardano(( block_info, diff --git a/modules/spo_state/src/state.rs b/modules/spo_state/src/state.rs index 84a4f566..5f9f617a 100644 --- a/modules/spo_state/src/state.rs +++ b/modules/spo_state/src/state.rs @@ -315,6 +315,7 @@ impl State { ) { if self.spos.contains_key(®.operator) { debug!( + epoch = self.epoch, block = block.number, "New pending SPO update {} {:?}", hex::encode(®.operator), @@ -323,6 +324,7 @@ impl State { self.pending_updates.insert(reg.operator.clone(), reg.clone()); } else { debug!( + epoch = self.epoch, block = block.number, "Registering SPO {} {:?}", hex::encode(®.operator), From 97599d33c315a4013bee5cf1d4975a429a3b0774 Mon Sep 17 00:00:00 2001 From: Paul Clark Date: Tue, 28 Oct 2025 17:50:04 +0000 Subject: [PATCH 2/3] cargo fmt --- codec/src/map_parameters.rs | 3 ++- modules/accounts_state/src/rewards.rs | 13 +++++++++---- modules/accounts_state/src/snapshot.rs | 10 +++++++--- modules/accounts_state/src/state.rs | 2 -- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/codec/src/map_parameters.rs b/codec/src/map_parameters.rs index d79a6959..4d8693fd 100644 --- a/codec/src/map_parameters.rs +++ b/codec/src/map_parameters.rs @@ -382,7 +382,8 @@ pub fn map_certificate( // a validation failure, but clearly wasn't! reward_account: StakeAddress::new( StakeAddress::from_binary(reward_account)?.payload, - network_id.clone()), + network_id.clone(), + ), pool_owners: pool_owners .into_iter() .map(|v| { diff --git a/modules/accounts_state/src/rewards.rs b/modules/accounts_state/src/rewards.rs index 1bf4cf0a..6daaaa0d 100644 --- a/modules/accounts_state/src/rewards.rs +++ b/modules/accounts_state/src/rewards.rs @@ -131,15 +131,20 @@ pub fn calculate_rewards( // Also, to handle the early Shelley timing bug, we allow it if it was registered // during the current epoch if !pay_to_pool_reward_account { - debug!("Checking old reward account {} for late registration", - staking_spo.reward_account); + debug!( + "Checking old reward account {} for late registration", + staking_spo.reward_account + ); // Note we use the staking reward account - it could have changed pay_to_pool_reward_account = registrations.contains(&staking_spo.reward_account); if pay_to_pool_reward_account { - info!("SPO {}'s reward account {} was registered in this epoch", - hex::encode(operator_id), staking_spo.reward_account); + info!( + "SPO {}'s reward account {} was registered in this epoch", + hex::encode(operator_id), + staking_spo.reward_account + ); } } diff --git a/modules/accounts_state/src/snapshot.rs b/modules/accounts_state/src/snapshot.rs index 8b2e347c..d6cb316b 100644 --- a/modules/accounts_state/src/snapshot.rs +++ b/modules/accounts_state/src/snapshot.rs @@ -95,9 +95,13 @@ impl Snapshot { .unwrap_or(false), None => false, }; - debug!(epoch, previous_epoch=two_previous_snapshot.epoch, - "Two previous reward account for SPO {} registered: {}", - hex::encode(spo_id), two_previous_reward_account_is_registered); + debug!( + epoch, + previous_epoch = two_previous_snapshot.epoch, + "Two previous reward account for SPO {} registered: {}", + hex::encode(spo_id), + two_previous_reward_account_is_registered + ); // Add the new one snapshot.spos.insert( diff --git a/modules/accounts_state/src/state.rs b/modules/accounts_state/src/state.rs index 3395bea4..db15508d 100644 --- a/modules/accounts_state/src/state.rs +++ b/modules/accounts_state/src/state.rs @@ -712,7 +712,6 @@ impl State { /// Handle an SPOStateMessage with the full set of SPOs valid at the end of the last /// epoch pub fn handle_spo_state(&mut self, spo_msg: &SPOStateMessage) -> Result<()> { - // Capture current SPOs, mapped by operator ID let new_spos: OrdMap = spo_msg.spos.iter().cloned().map(|spo| (spo.operator.clone(), spo)).collect(); @@ -795,7 +794,6 @@ impl State { /// Register a stake address, with a specified deposit if known fn register_stake_address(&mut self, stake_address: &StakeAddress, deposit: Option) { - debug!("Register stake address {stake_address}"); // Stake addresses can be registered after being used in UTXOs let mut stake_addresses = self.stake_addresses.lock().unwrap(); From 3421f7926205653911d929ccc09a953def8f8d6b Mon Sep 17 00:00:00 2001 From: Paul Clark Date: Tue, 28 Oct 2025 18:01:23 +0000 Subject: [PATCH 3/3] Fix to use StakeAddress credential after merge --- codec/src/map_parameters.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codec/src/map_parameters.rs b/codec/src/map_parameters.rs index 4d8693fd..7ad92af1 100644 --- a/codec/src/map_parameters.rs +++ b/codec/src/map_parameters.rs @@ -381,7 +381,7 @@ pub fn map_certificate( // reward account (e1). Feels like this should have been // a validation failure, but clearly wasn't! reward_account: StakeAddress::new( - StakeAddress::from_binary(reward_account)?.payload, + StakeAddress::from_binary(reward_account)?.credential, network_id.clone(), ), pool_owners: pool_owners