[Staking] Patch candidate bond more to update CandidatePool + hotfix extrinsic to fix incorrect state

pallets/parachain-staking/src/lib.rs

@@ -2114,6 +2114,9 @@ pub mod pallet {
 			let collator = ensure_signed(origin)?;
 			let mut state = <CandidateState<T>>::get(&collator).ok_or(Error::<T>::CandidateDNE)?;
 			state.bond_more::<T>(more)?;
+			if state.is_active() {
+				Self::update_active(collator.clone(), state.total_counted);
+			}
 			<CandidateState<T>>::insert(&collator, state);
 			Ok(().into())
 		}

pallets/parachain-staking/src/migrations.rs

@@ -36,6 +36,35 @@ use frame_support::{
 };
 use sp_std::{collections::btree_map::BTreeMap, vec::Vec};
 
+/// Migration to patch increase candidate bond not updating candidate pool
+pub struct UpdateCandidatePoolWithTotalCounted<T>(PhantomData<T>);
+impl<T: Config> OnRuntimeUpgrade for UpdateCandidatePoolWithTotalCounted<T> {
+	fn on_runtime_upgrade() -> Weight {
+		let (mut reads, mut writes) = (0u64, 0u64);
+		for (account, state) in <CandidateState<T>>::iter() {
+			reads += 1u64;
+			if state.is_active() {
+				Pallet::<T>::update_active(account.clone(), state.total_counted);
+				writes += 1u64;
+			}
+		}
+		let weight = T::DbWeight::get();
+		// 10% of the max block weight as safety margin for computation
+		weight.reads(reads) + weight.writes(writes) + 50_000_000_000
+	}
+	#[cfg(feature = "try-runtime")]
+	fn pre_upgrade() -> Result<(), &'static str> {
+		// trivial
+		Ok(())
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade() -> Result<(), &'static str> {
+		// trivial
+		Ok(())
+	}
+}
+
 /// Migration to properly increase maximum delegations per collator
 /// This migration can be used to recompute the top and bottom delegations whenever
 /// MaxDelegatorsPerCandidate changes (works for decrease as well)

pallets/parachain-staking/src/mock.rs

@@ -236,7 +236,6 @@ pub(crate) fn roll_one_block() -> u64 {
 	System::on_initialize(System::block_number());
 	Balances::on_initialize(System::block_number());
 	Stake::on_initialize(System::block_number());
-
 	System::block_number()
 }
 

pallets/parachain-staking/src/tests.rs

@@ -629,13 +629,8 @@ fn join_candidates_adds_to_candidate_pool() {
 			assert!(Stake::candidate_pool().0.is_empty());
 			assert_ok!(Stake::join_candidates(Origin::signed(1), 10u128, 0u32));
 			let candidate_pool = Stake::candidate_pool();
-			assert_eq!(
-				candidate_pool.0[0],
-				Bond {
-					owner: 1,
-					amount: 10u128
-				}
-			);
+			assert_eq!(candidate_pool.0[0].owner, 1);
+			assert_eq!(candidate_pool.0[0].amount, 10);
 		});
 }
 
@@ -960,13 +955,8 @@ fn cancel_leave_candidates_adds_to_candidate_pool() {
 		.execute_with(|| {
 			assert_ok!(Stake::schedule_leave_candidates(Origin::signed(1), 1u32));
 			assert_ok!(Stake::cancel_leave_candidates(Origin::signed(1), 1));
-			assert_eq!(
-				Stake::candidate_pool().0[0],
-				Bond {
-					owner: 1,
-					amount: 10
-				}
-			);
+			assert_eq!(Stake::candidate_pool().0[0].owner, 1);
+			assert_eq!(Stake::candidate_pool().0[0].amount, 10);
 		});
 }
 
@@ -1062,13 +1052,8 @@ fn go_online_adds_to_candidate_pool() {
 			assert_ok!(Stake::go_offline(Origin::signed(1)));
 			assert!(Stake::candidate_pool().0.is_empty());
 			assert_ok!(Stake::go_online(Origin::signed(1)));
-			assert_eq!(
-				Stake::candidate_pool().0[0],
-				Bond {
-					owner: 1,
-					amount: 20
-				}
-			);
+			assert_eq!(Stake::candidate_pool().0[0].owner, 1);
+			assert_eq!(Stake::candidate_pool().0[0].amount, 20);
 		});
 }
 
@@ -1192,21 +1177,11 @@ fn candidate_bond_more_updates_candidate_pool() {
 		.with_candidates(vec![(1, 20)])
 		.build()
 		.execute_with(|| {
-			assert_eq!(
-				Stake::candidate_pool().0[0],
-				Bond {
-					owner: 1,
-					amount: 20
-				}
-			);
+			assert_eq!(Stake::candidate_pool().0[0].owner, 1);
+			assert_eq!(Stake::candidate_pool().0[0].amount, 20);
 			assert_ok!(Stake::candidate_bond_more(Origin::signed(1), 30));
-			assert_eq!(
-				Stake::candidate_pool().0[0],
-				Bond {
-					owner: 1,
-					amount: 50
-				}
-			);
+			assert_eq!(Stake::candidate_pool().0[0].owner, 1);
+			assert_eq!(Stake::candidate_pool().0[0].amount, 50);
 		});
 }
 
@@ -1367,23 +1342,13 @@ fn execute_candidate_bond_less_updates_candidate_pool() {
 		.with_candidates(vec![(1, 30)])
 		.build()
 		.execute_with(|| {
-			assert_eq!(
-				Stake::candidate_pool().0[0],
-				Bond {
-					owner: 1,
-					amount: 30
-				}
-			);
+			assert_eq!(Stake::candidate_pool().0[0].owner, 1);
+			assert_eq!(Stake::candidate_pool().0[0].amount, 30);
 			assert_ok!(Stake::schedule_candidate_bond_less(Origin::signed(1), 10));
 			roll_to(10);
 			assert_ok!(Stake::execute_candidate_bond_less(Origin::signed(1), 1));
-			assert_eq!(
-				Stake::candidate_pool().0[0],
-				Bond {
-					owner: 1,
-					amount: 20
-				}
-			);
+			assert_eq!(Stake::candidate_pool().0[0].owner, 1);
+			assert_eq!(Stake::candidate_pool().0[0].amount, 20);
 		});
 }
 
@@ -1477,13 +1442,8 @@ fn delegate_updates_delegator_state() {
 			assert_ok!(Stake::delegate(Origin::signed(2), 1, 10, 0, 0));
 			let delegator_state = Stake::delegator_state(2).expect("just delegated => exists");
 			assert_eq!(delegator_state.total, 10);
-			assert_eq!(
-				delegator_state.delegations.0[0],
-				Bond {
-					owner: 1,
-					amount: 10
-				}
-			);
+			assert_eq!(delegator_state.delegations.0[0].owner, 1);
+			assert_eq!(delegator_state.delegations.0[0].amount, 10);
 		});
 }
 
@@ -1502,13 +1462,8 @@ fn delegate_updates_collator_state() {
 			let candidate_state = Stake::candidate_state(1).expect("just delegated => exists");
 			assert_eq!(candidate_state.total_backing, 40);
 			assert_eq!(candidate_state.total_counted, 40);
-			assert_eq!(
-				candidate_state.top_delegations[0],
-				Bond {
-					owner: 2,
-					amount: 10
-				}
-			);
+			assert_eq!(candidate_state.top_delegations[0].owner, 2);
+			assert_eq!(candidate_state.top_delegations[0].amount, 10);
 		});
 }
 
@@ -1794,13 +1749,8 @@ fn execute_leave_delegators_removes_delegations_from_collator_state() {
 			for i in 2..6 {
 				let candidate_state =
 					Stake::candidate_state(i).expect("initialized in ext builder");
-				assert_eq!(
-					candidate_state.top_delegations[0],
-					Bond {
-						owner: 1,
-						amount: 10
-					}
-				);
+				assert_eq!(candidate_state.top_delegations[0].owner, 1);
+				assert_eq!(candidate_state.top_delegations[0].amount, 10);
 				assert_eq!(candidate_state.delegators.0[0], 1);
 				assert_eq!(candidate_state.total_backing, 30);
 			}
@@ -2051,19 +2001,21 @@ fn delegator_bond_more_updates_candidate_state_top_delegations() {
 		.build()
 		.execute_with(|| {
 			assert_eq!(
-				Stake::candidate_state(1).expect("exists").top_delegations[0],
-				Bond {
-					owner: 2,
-					amount: 10
-				}
+				Stake::candidate_state(1).expect("exists").top_delegations[0].owner,
+				2
+			);
+			assert_eq!(
+				Stake::candidate_state(1).expect("exists").top_delegations[0].amount,
+				10
 			);
 			assert_ok!(Stake::delegator_bond_more(Origin::signed(2), 1, 5));
 			assert_eq!(
-				Stake::candidate_state(1).expect("exists").top_delegations[0],
-				Bond {
-					owner: 2,
-					amount: 15
-				}
+				Stake::candidate_state(1).expect("exists").top_delegations[0].owner,
+				2
+			);
+			assert_eq!(
+				Stake::candidate_state(1).expect("exists").top_delegations[0].amount,
+				15
 			);
 		});
 }
@@ -2085,22 +2037,32 @@ fn delegator_bond_more_updates_candidate_state_bottom_delegations() {
 			assert_eq!(
 				Stake::candidate_state(1)
 					.expect("exists")
-					.bottom_delegations[0],
-				Bond {
-					owner: 2,
-					amount: 10
-				}
+					.bottom_delegations[0]
+					.owner,
+				2
+			);
+			assert_eq!(
+				Stake::candidate_state(1)
+					.expect("exists")
+					.bottom_delegations[0]
+					.amount,
+				10
 			);
 			assert_ok!(Stake::delegator_bond_more(Origin::signed(2), 1, 5));
 			assert_last_event!(MetaEvent::Stake(Event::DelegationIncreased(2, 1, 5, false)));
 			assert_eq!(
 				Stake::candidate_state(1)
 					.expect("exists")
-					.bottom_delegations[0],
-				Bond {
-					owner: 2,
-					amount: 15
-				}
+					.bottom_delegations[0]
+					.owner,
+				2
+			);
+			assert_eq!(
+				Stake::candidate_state(1)
+					.expect("exists")
+					.bottom_delegations[0]
+					.amount,
+				15
 			);
 		});
 }
@@ -2624,21 +2586,23 @@ fn execute_delegator_bond_less_updates_candidate_state() {
 		.build()
 		.execute_with(|| {
 			assert_eq!(
-				Stake::candidate_state(1).expect("exists").top_delegations[0],
-				Bond {
-					owner: 2,
-					amount: 10
-				}
+				Stake::candidate_state(1).expect("exists").top_delegations[0].owner,
+				2
+			);
+			assert_eq!(
+				Stake::candidate_state(1).expect("exists").top_delegations[0].amount,
+				10
 			);
 			assert_ok!(Stake::schedule_delegator_bond_less(Origin::signed(2), 1, 5));
 			roll_to(10);
 			assert_ok!(Stake::execute_delegation_request(Origin::signed(2), 2, 1));
 			assert_eq!(
-				Stake::candidate_state(1).expect("exists").top_delegations[0],
-				Bond {
-					owner: 2,
-					amount: 5
-				}
+				Stake::candidate_state(1).expect("exists").top_delegations[0].owner,
+				2
+			);
+			assert_eq!(
+				Stake::candidate_state(1).expect("exists").top_delegations[0].amount,
+				5
 			);
 		});
 }

runtime/common/src/migrations.rs

@@ -25,14 +25,41 @@ use frame_support::{
 use pallet_author_mapping::{migrations::TwoXToBlake, Config as AuthorMappingConfig};
 use pallet_migrations::Migration;
 use parachain_staking::{
-	migrations::{IncreaseMaxDelegationsPerCandidate, PurgeStaleStorage, RemoveExitQueue},
+	migrations::{
+		IncreaseMaxDelegationsPerCandidate, PurgeStaleStorage, RemoveExitQueue,
+		UpdateCandidatePoolWithTotalCounted,
+	},
 	Config as ParachainStakingConfig,
 };
 use sp_std::{marker::PhantomData, prelude::*};
 
 /// This module acts as a registry where each migration is defined. Each migration should implement
 /// the "Migration" trait declared in the pallet-migrations crate.
 
+/// Staking increase max counted delegations per collator candidate
+pub struct ParachainStakingUpdateCandidatePool<T>(PhantomData<T>);
+impl<T: ParachainStakingConfig> Migration for ParachainStakingUpdateCandidatePool<T> {
+	fn friendly_name(&self) -> &str {
+		"MM_Parachain_Staking_UpdateCandidatePoolWithTotalCounted"
+	}
+
+	fn migrate(&self, _available_weight: Weight) -> Weight {
+		UpdateCandidatePoolWithTotalCounted::<T>::on_runtime_upgrade()
+	}
+
+	/// Run a standard pre-runtime test. This works the same way as in a normal runtime upgrade.
+	#[cfg(feature = "try-runtime")]
+	fn pre_upgrade(&self) -> Result<(), &'static str> {
+		UpdateCandidatePoolWithTotalCounted::<T>::pre_upgrade()
+	}
+
+	/// Run a standard post-runtime test. This works the same way as in a normal runtime upgrade.
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade(&self) -> Result<(), &'static str> {
+		UpdateCandidatePoolWithTotalCounted::<T>::post_upgrade()
+	}
+}
+
 /// Staking increase max counted delegations per collator candidate
 pub struct ParachainStakingIncreaseMaxDelegationsPerCandidate<T>(PhantomData<T>);
 impl<T: ParachainStakingConfig> Migration
@@ -190,6 +217,8 @@ where
 		// 	ParachainStakingManualExits::<Runtime>(Default::default());
 		let migration_parachain_staking_increase_max_delegations_per_candidate =
 			ParachainStakingIncreaseMaxDelegationsPerCandidate::<Runtime>(Default::default());
+		let migration_parachain_staking_update_candidate_pool =
+			ParachainStakingUpdateCandidatePool::<Runtime>(Default::default());
 
 		// TODO: this is a lot of allocation to do upon every get() call. this *should* be avoided
 		// except when pallet_migrations undergoes a runtime upgrade -- but TODO: review
@@ -204,6 +233,7 @@ where
 			// completed in runtime 1000
 			// Box::new(migration_parachain_staking_manual_exits),
 			Box::new(migration_parachain_staking_increase_max_delegations_per_candidate),
+			Box::new(migration_parachain_staking_update_candidate_pool),
 		]
 	}
 }