Repository: chainx-org/ChainX (language: rust)

Issue:
The balance locks not updated when staker unbonds the total stakes
https://github.com/chainx-org/ChainX/blob/6cbd6e24151af4aa2741b4a4bddf10dc02b0d6c2/xpallets/mining/staking/src/lib.rs#L785-L788

When the new locked value is zero, we should use `T::Currency::remove_lock()` instead of calling `T::Currency::set_lock()` as it's a no-op in this case. This bug causes these accounts' balance is still locked after applying the unlock operation by Block 169169:

```
5QWJ8mzw7vGSwAopszkBLuDNVVy8ERwkGNozqgdc7aamvpRC
5QhLJ775BZUoGfT7Y9wsh5oXFZWkNcFEBYsz46zZB5jTzb4S
5R7K9qZRcYPZHWGEWY5NTxi5y4dCGZiGREv31ksdJjs7EFzC
5RZvfgz2cHhS3EKpmaWxkJZEhqAoKTG11yFqY1ru4gMcEDTj
5RhYr9qj7sBbtcnhoAXSVihWWaUCAra2sUY7CQEw82JNHuQu
5RnQ3dEeg1LAAuGt4nAi53kQHXELXaUDtdshUyLVEdAM3CTS
5SPnyNTsa6eZWzP91ehdQJ5U7NPqbHaJyfZmmXuKEyHjeKAL
5SgEcwcq8v7i1X2ri61ezfTzYo8r4txffA673NFjQBaK3UPR
5Sgtpu5BabQX19Rd9bhQ91K1fH8ZhB6YDHY7DCncJxZKcwqp
5TBeDfFStXG5zeAmiEH9zsd2gEzNq1yGWMBZvXkzrWh89XPt
5TM3iCzL1oDns5yv1MhqjabswrtQnsMzgy6qiTuti9f6NAPU
5TsBCP8HqNDdNgZbKAJ6sb6WsSBXwCdB95bj8Caach4JPFcC
5UacrUrD38SvX8UJNW4nq1SKnjebSt1iBkptZ6awv24yLHMT
5Uk7dNSXBtn15dS5SbuoJ3Zng1x2UtUqGtCBeyhpnL3xR6ZH
5VGFb4aYvvkLkLpoXyH5Hy4yvg6ARCKrKJYxBj52sWsDbxao
```



Discussion / hints:

Fix patch:
diff --git a/runtime/chainx/src/lib.rs b/runtime/chainx/src/lib.rs
index f1fd4d5e7..6e00c24ed 100644
--- a/runtime/chainx/src/lib.rs
+++ b/runtime/chainx/src/lib.rs
@@ -104,7 +104,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("chainx"),
     impl_name: create_runtime_str!("chainx-net"),
     authoring_version: 1,
-    spec_version: 5,
+    spec_version: 6,
     impl_version: 1,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 1,
diff --git a/xpallets/mining/staking/src/lib.rs b/xpallets/mining/staking/src/lib.rs
index 51bba506f..f3cfa7872 100644
--- a/xpallets/mining/staking/src/lib.rs
+++ b/xpallets/mining/staking/src/lib.rs
@@ -549,6 +549,7 @@ decl_module! {
 
         #[weight = 10_000_000]
         fn force_reset_staking_lock(origin, accounts: Vec<T::AccountId>) {
+            ensure_root(origin)?;
             for who in accounts.iter() {
                 Locks::<T>::mutate(who, |locks| {
                     locks.remove(&LockedType::BondedWithdrawal);
@@ -557,6 +558,14 @@ decl_module! {
                 });
             }
         }
+
+        #[weight = 10_000_000]
+        fn force_set_lock(origin, new_locks: Vec<(T::AccountId, BalanceOf<T>)>) {
+            ensure_root(origin)?;
+            for (who, new_lock) in new_locks {
+                Self::set_lock(&who, new_lock);
+            }
+        }
     }
 }
 
@@ -781,10 +790,14 @@ impl<T: Trait> Module<T> {
         T::Currency::transfer(from, to, value, ExistenceRequirement::KeepAlive)
     }
 
-    /// Create/Update a new balance lock on account `who`.
+    /// Create/Update/Remove a new balance lock on account `who`.
     #[inline]
     fn set_lock(who: &T::AccountId, new_locked: BalanceOf<T>) {
-        T::Currency::set_lock(STAKING_ID, who, new_locked, WithdrawReasons::all());
+        if new_locked.is_zero() {
+            T::Currency::remove_lock(STAKING_ID, who);
+        } else {
+            T::Currency::set_lock(STAKING_ID, who, new_locked, WithdrawReasons::all());
+        }
     }
 
     fn purge_unlockings(who: &T::AccountId) {


Test patch:
diff --git a/xpallets/mining/staking/src/tests.rs b/xpallets/mining/staking/src/tests.rs
index 9ef28b199..a21ea2d65 100644
--- a/xpallets/mining/staking/src/tests.rs
+++ b/xpallets/mining/staking/src/tests.rs
@@ -365,14 +365,14 @@ fn withdraw_unbond_should_work() {
 
         t_system_block_number_inc(1);
 
-        assert_ok!(t_unbond(1, 2, 5));
-        let before_unbond = Balances::usable_balance(&1);
-        assert_eq!(Balances::usable_balance(&1), before_unbond);
+        let unbond_value = 10;
+
+        assert_ok!(t_unbond(1, 2, unbond_value));
 
         assert_eq!(
             <Nominations<Test>>::get(1, 2).unbonded_chunks,
             vec![Unbonded {
-                value: 5,
+                value: unbond_value,
                 locked_until: DEFAULT_BONDING_DURATION + 3
             }]
         );
@@ -386,9 +386,27 @@ fn withdraw_unbond_should_work() {
         t_system_block_number_inc(1);
 
         let before_withdraw_unbonded = Balances::usable_balance(&1);
-        assert_ok!(t_withdraw_unbonded(1, 2, 0),);
-        assert_eq!(Balances::usable_balance(&1), before_withdraw_unbonded + 5);
+        assert_ok!(t_withdraw_unbonded(1, 2, 0));
+        assert_eq!(
+            Balances::usable_balance(&1),
+            before_withdraw_unbonded + unbond_value
+        );
+
         assert_bonded_withdrawal_locks(1, 0);
+
+        // Unbond total stakes should work.
+        assert_ok!(t_unbond(1, 1, 10));
+        t_system_block_number_inc(DEFAULT_VALIDATOR_BONDING_DURATION + 1);
+        assert_ok!(t_withdraw_unbonded(1, 1, 0));
+        assert_eq!(
+            frame_system::Account::<Test>::get(&1).data,
+            pallet_balances::AccountData {
+                free: 100,
+                reserved: 0,
+                misc_frozen: 0,
+                fee_frozen: 0,
+            }
+        );
     });
 }