More tests & docs

Author: Dinonard

pallets/dapp-staking-v3/src/test/tests_types.rs

@@ -57,7 +57,7 @@ fn period_info_basic_checks() {
     assert_eq!(info.subperiod, Subperiod::Voting);
     assert_eq!(info.next_subperiod_start_era, next_subperiod_start_era);
 
-    // Voting period checks
+    // Voting subperiod checks
     assert!(!info.is_next_period(next_subperiod_start_era - 1));
     assert!(!info.is_next_period(next_subperiod_start_era));
     assert!(!info.is_next_period(next_subperiod_start_era + 1));
@@ -68,7 +68,7 @@ fn period_info_basic_checks() {
     ] {
         assert!(
             !info.is_next_period(era),
-            "Cannot trigger 'true' in the Voting period type."
+            "Cannot trigger 'true' in the Voting subperiod type."
         );
     }
 
@@ -520,7 +520,7 @@ fn account_ledger_add_stake_amount_basic_example_with_different_subperiods_works
     assert!(acc_ledger.staked.is_empty());
     assert!(acc_ledger.staked_future.is_none());
 
-    // 1st scenario - stake some amount in Voting period, and ensure values are as expected.
+    // 1st scenario - stake some amount in Voting subperiod, and ensure values are as expected.
     let era_1 = 1;
     let period_1 = 1;
     let period_info_1 = PeriodInfo {
@@ -1775,7 +1775,7 @@ fn era_info_stake_works() {
     // Sanity check
     assert!(era_info.total_locked.is_zero());
 
-    // Add some voting period stake
+    // Add some voting subperiod stake
     let vp_stake_amount = 7;
     era_info.add_stake_amount(vp_stake_amount, Subperiod::Voting);
     assert_eq!(era_info.total_staked_amount_next_era(), vp_stake_amount);
@@ -2080,7 +2080,7 @@ fn singular_staking_info_unstake_during_voting_is_ok() {
     let vote_stake_amount_1 = 11;
     staking_info.stake(vote_stake_amount_1, era_1, Subperiod::Voting);
 
-    // Unstake some amount during `Voting` period, loyalty should remain as expected.
+    // 1. Unstake some amount during `Voting` period, loyalty should remain as expected.
     let unstake_amount_1 = 5;
     assert_eq!(
         staking_info.unstake(unstake_amount_1, era_1, Subperiod::Voting),
@@ -2097,7 +2097,7 @@ fn singular_staking_info_unstake_during_voting_is_ok() {
         "Stake era should remain valid."
     );
 
-    // Fully unstake, attempting to underflow, and ensure loyalty flag has been removed.
+    // 2. Fully unstake, attempting to underflow, and ensure loyalty flag has been removed.
     let era_2 = era_1 + 2;
     let remaining_stake = staking_info.total_staked_amount();
     assert_eq!(
@@ -2151,8 +2151,21 @@ fn singular_staking_info_unstake_during_bep_is_ok() {
         "Stake era should remain valid."
     );
 
-    // 2nd scenario - unstake all of the amount staked during B&E period, and then some more.
-    // The point is to take a chunk from the voting period stake too.
+    // 2nd scenario - Ensure that staked amount is larger than the previous stake amount, and then
+    // unstake enough to result in some overflow of the stake delta.
+    let bep_stake_amount_2 = 13;
+    staking_info.stake(bep_stake_amount_2, era_1, Subperiod::BuildAndEarn);
+    let delta = staking_info.staked.total() - staking_info.previous_staked.total();
+    let overflow = 1;
+    let unstake_2 = delta + overflow;
+
+    assert_eq!(
+        staking_info.unstake(unstake_2, era_1, Subperiod::BuildAndEarn),
+        vec![(era_1, overflow), (era_1 + 1, unstake_2)]
+    );
+
+    // 3rd scenario - unstake all of the amount staked during B&E period, and then some more.
+    // The point is to take a chunk from the voting subperiod stake too.
     let current_total_stake = staking_info.total_staked_amount();
     let current_bep_stake = staking_info.staked_amount(Subperiod::BuildAndEarn);
     let voting_stake_overflow = 2;
@@ -2161,7 +2174,7 @@ fn singular_staking_info_unstake_during_bep_is_ok() {
 
     assert_eq!(
         staking_info.unstake(unstake_2, era_2, Subperiod::BuildAndEarn),
-        // Both amounts are the same since current stake is less than the previous one
+        // Both amounts are the same since previous staked era is less than last staked era - 1
         vec![(era_2 - 1, unstake_2), (era_2, unstake_2)]
     );
     assert_eq!(
@@ -2177,7 +2190,7 @@ fn singular_staking_info_unstake_during_bep_is_ok() {
         .is_zero());
     assert!(
         !staking_info.is_loyal(),
-        "Loyalty flag should have been removed due to non-zero voting period unstake"
+        "Loyalty flag should have been removed due to non-zero voting subperiod unstake"
     );
     assert_eq!(staking_info.era(), era_2);
 }

pallets/dapp-staking-v3/src/types.rs

@@ -1015,40 +1015,67 @@ impl SingularStakingInfo {
     ///
     /// Returns a vector of `(era, amount)` pairs, where `era` is the era in which the unstake happened,
     /// and the amount is the corresponding amount.
+    ///
+    /// ### NOTE
+    /// `SingularStakingInfo` always aims to keep track of the staked amount between two consecutive eras.
+    /// This means that the returned value will at most cover two eras - the last staked era, and the one before it.
+    ///
+    /// Last staked era can be the current era, or the era after.
+    /// If after the unstake operation, delta between previous & current era is larger than 1, previous staked entry is ignored.
+    ///
+    /// #### Example (simplified)
+    ///
+    /// 1. previous_staked: (era: 4, amount: 50), staked: (era: 5, amount: 100)
+    /// 2. unstake 30 in era 6 is called
+    /// 3. Return value is: (era: 5, amount: 30), (era: 6, amount: 30)
+    /// 4. previous_staked: (era: 5, amount: 70), (era: 6, amount: 70)
+    ///
+    /// In case same example is repeated, but unstake is done in era 5:
+    ///
+    /// 1. previous_staked: (era: 4, amount: 50), staked: (era: 5, amount: 100)
+    /// 2. unstake 30 in era 5 is called
+    /// 3. Return value is: (era: 5, amount: 30)
+    /// 4. previous_staked: (era: 4, amount: 50), (era: 5, amount: 70)
+    ///
     pub fn unstake(
         &mut self,
         amount: Balance,
         current_era: EraNumber,
         subperiod: Subperiod,
     ) -> Vec<(EraNumber, Balance)> {
         let mut result = Vec::new();
-
-        // 0. Prepare values for further calculations
-        let snapshot = self.staked;
-        let stake_delta = self
-            .staked
-            .total()
-            .saturating_sub(self.previous_staked.total());
+        let staked_snapshot = self.staked;
 
         // 1. Modify current staked amount
         self.staked.subtract(amount);
-        let unstaked_amount = snapshot.total().saturating_sub(self.staked.total());
+        let unstaked_amount = staked_snapshot.total().saturating_sub(self.staked.total());
         self.staked.era = self.staked.era.max(current_era);
         result.push((self.staked.era, unstaked_amount));
 
+        // 2. Calculate previous stake delta.
+        // In case new stake era is exactly one era after the previous stake era, we can calculate this as a delta.
+        // Otherwise, if it's further far away in the past, the previous era stake is equal to the snapshot.
+        let stake_delta = if self.staked.era == self.previous_staked.era.saturating_add(1) {
+            staked_snapshot
+                .total()
+                .saturating_sub(self.previous_staked.total())
+        } else {
+            Balance::zero()
+        };
+
         // 2. Update loyal staker flag accordingly
         self.loyal_staker = self.loyal_staker
             && match subperiod {
                 Subperiod::Voting => !self.staked.voting.is_zero(),
-                Subperiod::BuildAndEarn => self.staked.voting == snapshot.voting,
+                Subperiod::BuildAndEarn => self.staked.voting == staked_snapshot.voting,
             };
 
         // 3. Move over the snapshot to the previous snapshot field and make sure
         // to update era in case something was staked before.
         if stake_delta.is_zero() {
             self.previous_staked = Default::default();
         } else {
-            self.previous_staked = snapshot;
+            self.previous_staked = staked_snapshot;
             self.previous_staked.era = self.staked.era.saturating_sub(1);
         }