fix(stakes): fix mining age reset function and add test

Author: drcpu-github

data_structures/src/staking/mod.rs

@@ -20,6 +20,7 @@ pub mod prelude {
 }
 
 #[cfg(test)]
+/// Test module
 pub mod test {
     use super::prelude::*;
 
@@ -33,32 +34,32 @@ pub mod test {
         stakes.add_stake("Alpha", 2, 0, MIN_STAKE_NANOWITS).unwrap();
 
         // Nobody holds any power just yet
-        let rank = stakes.rank(Capability::Mining, 0).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 0).collect::<Vec<_>>();
         assert_eq!(rank, vec![("Alpha".into(), 0)]);
 
         // One epoch later, Alpha starts to hold power
-        let rank = stakes.rank(Capability::Mining, 1).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 1).collect::<Vec<_>>();
         assert_eq!(rank, vec![("Alpha".into(), 2)]);
 
         // Beta stakes 5 @ epoch 10
         stakes.add_stake("Beta", 5, 10, MIN_STAKE_NANOWITS).unwrap();
 
         // Alpha is still leading, but Beta has scheduled its takeover
-        let rank = stakes.rank(Capability::Mining, 10).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 10).collect::<Vec<_>>();
         assert_eq!(rank, vec![("Alpha".into(), 20), ("Beta".into(), 0)]);
 
         // Beta eventually takes over after epoch 16
-        let rank = stakes.rank(Capability::Mining, 16).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 16).collect::<Vec<_>>();
         assert_eq!(rank, vec![("Alpha".into(), 32), ("Beta".into(), 30)]);
-        let rank = stakes.rank(Capability::Mining, 17).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 17).collect::<Vec<_>>();
         assert_eq!(rank, vec![("Beta".into(), 35), ("Alpha".into(), 34)]);
 
         // Gamma should never take over, even in a million epochs, because it has only 1 coin
         stakes
             .add_stake("Gamma", 1, 30, MIN_STAKE_NANOWITS)
             .unwrap();
         let rank = stakes
-            .rank(Capability::Mining, 1_000_000)
+            .by_rank(Capability::Mining, 1_000_000)
             .collect::<Vec<_>>();
         assert_eq!(
             rank,
@@ -73,7 +74,7 @@ pub mod test {
         stakes
             .add_stake("Delta", 1_000, 50, MIN_STAKE_NANOWITS)
             .unwrap();
-        let rank = stakes.rank(Capability::Mining, 50).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 50).collect::<Vec<_>>();
         assert_eq!(
             rank,
             vec![
@@ -83,7 +84,7 @@ pub mod test {
                 ("Delta".into(), 0)
             ]
         );
-        let rank = stakes.rank(Capability::Mining, 51).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 51).collect::<Vec<_>>();
         assert_eq!(
             rank,
             vec![
@@ -96,7 +97,7 @@ pub mod test {
 
         // If Alpha removes all of its stake, it should immediately disappear
         stakes.remove_stake("Alpha", 2, MIN_STAKE_NANOWITS).unwrap();
-        let rank = stakes.rank(Capability::Mining, 51).collect::<Vec<_>>();
+        let rank = stakes.by_rank(Capability::Mining, 51).collect::<Vec<_>>();
         assert_eq!(
             rank,
             vec![

data_structures/src/staking/stakes.rs

@@ -322,12 +322,9 @@ where
         &self,
         capability: Capability,
         current_epoch: Epoch,
-    ) -> impl Iterator<Item = (StakeKey<Address>, Power)> + '_ {
+    ) -> impl Iterator<Item = (StakeKey<Address>, Power)> + Clone + '_ {
         self.by_key
             .iter()
-            .filter(|(_, sync_entry)| {
-                sync_entry.read_value().epochs.get(capability) <= current_epoch
-            })
             .map(move |(key, entry)| {
                 (key.clone(), entry.read_value().power(capability, current_epoch))
             })
@@ -428,10 +425,11 @@ where
         let validator = validator.into();
 
         // order mining stake entries by rank, as for given current_epoch:
-        let mut by_rank = self.by_rank(Capability::Mining, current_epoch);
+        // let stakes_clone = self.clone();
+        let by_rank = self.by_rank(Capability::Mining, current_epoch);
 
         // locate first entry whose validator matches the one searched for:
-        let winner_rank = by_rank.position(|(key, _)| key.validator == validator);
+        let winner_rank = by_rank.clone().position(|(key, _)| key.validator == validator);
 
         if let Some(winner_rank) = winner_rank {
             let stakers: Vec<StakeKey<Address>> = by_rank
@@ -444,9 +442,9 @@ where
                 let stake_entry = self.by_key.get_mut(key);
                 if let Some(stake_entry) = stake_entry {
                     let penalty_epochs = Epoch::from((1 + winner_rank - index) as u32);
-                    log::info!(
-                        "Resetting and disabling mining power of {} (ranked as #{}) during +{} epochs", 
-                        key, index + 1, penalty_epochs
+                    log::debug!(
+                        "Resetting mining power of {} (ranked as #{}) during +{} epochs to {}",
+                        key, index + 1, penalty_epochs, current_epoch + penalty_epochs
                     );
                     stake_entry
                         .value
@@ -1477,51 +1475,104 @@ mod tests {
     }
 
     #[test]
-    fn test_stakes_cloneability() {
+    fn test_reset_mining_age() {
         // First, lets create a setup with a few stakers
         let mut stakes = StakesTester::default();
         let alice = "Alice";
         let bob = "Bob";
         let charlie = "Charlie";
         let david = "David";
 
+        let alice_alice = (alice, alice);
+        let bob_alice = (bob, alice);
+        let charlie_charlie = (charlie, charlie);
+        let david_charlie = (david, charlie);
+
         // Add some stake and verify the power
         stakes
-            .add_stake((alice, charlie), 10, 0, MIN_STAKE_NANOWITS)
+            .add_stake(alice_alice, 10, 0, MIN_STAKE_NANOWITS)
+            .unwrap();
+        stakes
+            .add_stake(bob_alice, 20, 0, MIN_STAKE_NANOWITS)
             .unwrap();
         stakes
-            .add_stake((bob, david), 20, 10, MIN_STAKE_NANOWITS)
+            .add_stake(charlie_charlie, 30, 10, MIN_STAKE_NANOWITS)
             .unwrap();
-        assert_eq!(stakes.query_power(alice, Capability::Mining, 30), Ok(300));
-        assert_eq!(stakes.query_power(bob, Capability::Mining, 30), Ok(400));
+        stakes
+            .add_stake(david_charlie, 40, 10, MIN_STAKE_NANOWITS)
+            .unwrap();
+        assert_eq!(
+            stakes.by_rank(Capability::Mining, 30).collect::<Vec<_>>(),
+            [
+                (david_charlie.into(), 800),
+                (charlie_charlie.into(), 600),
+                (bob_alice.into(), 600),
+                (alice_alice.into(), 300),
+            ]
+        );
+
+        stakes.reset_mining_age(david, 30).unwrap();
 
-        // Clone the stakes structure and verify the power
-        let cloned_stakes = stakes.clone();
         assert_eq!(
-            cloned_stakes.query_power(alice, Capability::Mining, 30),
-            Ok(300)
+            stakes.by_rank(Capability::Mining, 31).collect::<Vec<_>>(),
+            [
+                (charlie_charlie.into(), 630),
+                (bob_alice.into(), 620),
+                (alice_alice.into(), 310),
+                (david_charlie.into(), 0),
+            ]
         );
+
         assert_eq!(
-            cloned_stakes.query_power(bob, Capability::Mining, 30),
-            Ok(400)
+            stakes.by_rank(Capability::Mining, 50).collect::<Vec<_>>(),
+            [
+                (charlie_charlie.into(), 1_200),
+                (bob_alice.into(), 1_000),
+                (david_charlie.into(), 760),
+                (alice_alice.into(), 500),
+            ]
         );
 
-        // Reset age and verify power
-        stakes.reset_age(alice, Capability::Mining, 25, 1).unwrap();
-        stakes.reset_age(bob, Capability::Mining, 30, 1).unwrap();
+        stakes.reset_mining_age(david, 50).unwrap();
 
-        // Power of validators in stakes should have changed
-        assert_eq!(stakes.query_power(alice, Capability::Mining, 30), Ok(50));
-        assert_eq!(stakes.query_power(bob, Capability::Mining, 30), Ok(0));
+        assert_eq!(
+            stakes.by_rank(Capability::Mining, 51).collect::<Vec<_>>(),
+            [
+                (alice_alice.into(), 510),
+                (david_charlie.into(), 0),
+                (charlie_charlie.into(), 0),
+                (bob_alice.into(), 0),
+            ]
+        );
 
-        // Power of validators in cloned_stakes should have changed
         assert_eq!(
-            cloned_stakes.query_power(alice, Capability::Mining, 30),
-            Ok(300)
+            stakes.by_rank(Capability::Mining, 52).collect::<Vec<_>>(),
+            [
+                (alice_alice.into(), 520),
+                (david_charlie.into(), 40),
+                (charlie_charlie.into(), 0),
+                (bob_alice.into(), 0),
+            ]
         );
+
         assert_eq!(
-            cloned_stakes.query_power(bob, Capability::Mining, 30),
-            Ok(400)
+            stakes.by_rank(Capability::Mining, 53).collect::<Vec<_>>(),
+            [
+                (alice_alice.into(), 530),
+                (david_charlie.into(), 80),
+                (bob_alice.into(), 20),
+                (charlie_charlie.into(), 0),
+            ]
+        );
+
+        assert_eq!(
+            stakes.by_rank(Capability::Mining, 54).collect::<Vec<_>>(),
+            [
+                (alice_alice.into(), 540),
+                (david_charlie.into(), 120),
+                (bob_alice.into(), 40),
+                (charlie_charlie.into(), 30),
+            ]
         );
     }
 }