Merge pull request #310 from input-output-hk/prc/303-missing-spo-rewards

Force networkId on pool reward accounts

Author: sandtreader

codec/src/map_parameters.rs

@@ -376,7 +376,14 @@ 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)?.credential,
+                            network_id.clone(),
+                        ),
                         pool_owners: pool_owners
                             .into_iter()
                             .map(|v| {

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()))
     }
 }
 

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,21 @@ 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 +173,7 @@ pub fn calculate_rewards(
             }
         } else {
             info!(
-                "Reward account for SPO {} was deregistered",
+                "Reward account for SPO {} isn't registered",
                 hex::encode(operator_id)
             )
         }

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,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
+            );
 
             // Add the new one
             snapshot.spos.insert(
@@ -128,7 +135,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
                         );

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
@@ -734,12 +734,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 +754,7 @@ impl State {
                         pledge = spo.pledge,
                         cost = spo.cost,
                         margin = ?spo.margin,
+                        reward = %spo.reward_account,
                         "Registered new SPO {}",
                         hex::encode(id)
                     );
@@ -791,6 +794,7 @@ impl State {
 
     /// Register a stake address, with a specified deposit if known
     fn register_stake_address(&mut self, stake_address: &StakeAddress, deposit: Option<Lovelace>) {
+        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 +823,8 @@ impl State {
 
     /// Deregister a stake address, with specified refund if known
     fn deregister_stake_address(&mut self, stake_address: &StakeAddress, refund: Option<Lovelace>) {
+        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) {

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,

modules/spo_state/src/state.rs

@@ -315,6 +315,7 @@ impl State {
     ) {
         if self.spos.contains_key(&reg.operator) {
             debug!(
+                epoch = self.epoch,
                 block = block.number,
                 "New pending SPO update {} {:?}",
                 hex::encode(&reg.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(&reg.operator),