Staking V3 review

pallet/account-migration/Cargo.toml

@@ -14,7 +14,6 @@ scale-info  = { workspace = true }
 
 # darwinia
 darwinia-deposit = { workspace = true }
-darwinia-staking = { workspace = true }
 dc-primitives    = { workspace = true }
 
 # polkadot-sdk
@@ -43,7 +42,6 @@ std = [
 
 	# darwinia
 	"darwinia-deposit/std",
-	"darwinia-staking/std",
 	"dc-primitives/std",
 
 	# polkadot-sdk
@@ -62,7 +60,6 @@ std = [
 runtime-benchmarks = [
 	# darwinia
 	"darwinia-deposit/runtime-benchmarks",
-	"darwinia-staking/runtime-benchmarks",
 
 	# polkadot-sdk
 	"frame-benchmarking/runtime-benchmarks",
@@ -76,7 +73,6 @@ runtime-benchmarks = [
 try-runtime = [
 	# darwinia
 	"darwinia-deposit/try-runtime",
-	"darwinia-staking/try-runtime",
 
 	# polkadot-sdk
 	"frame-support/try-runtime",

pallet/account-migration/src/lib.rs

@@ -24,7 +24,7 @@
 //! These two algorithm are not compatible.
 //! Thus, an account migration is required.
 //!
-//! ## Technical detail
+//! ## Technical Detail
 //!
 //! Users must send an extrinsic themselves to migrate their account(s).
 //! This extrinsic should be unsigned, the reason is the same as `pallet-claims`.
@@ -56,7 +56,6 @@ pub use weights::WeightInfo;
 
 // darwinia
 use darwinia_deposit::{Deposit, DepositId};
-use darwinia_staking::Ledger;
 use dc_primitives::{AccountId as AccountId20, AssetId, Balance, BlockNumber, Nonce};
 // polkadot-sdk
 use frame_support::{
@@ -90,14 +89,11 @@ pub mod pallet {
 	pub(crate) const E_ACCOUNT_NOT_FOUND: u8 = 1;
 	/// Invalid signature.
 	pub(crate) const E_INVALID_SIGNATURE: u8 = 2;
-	/// Duplicative submission.
-	pub(crate) const E_DUPLICATIVE_SUBMISSION: u8 = 3;
+	/// Duplicated submission.
+	pub(crate) const E_DUPLICATED_SUBMISSION: u8 = 3;
 	/// The account is not a member of the multisig.
 	pub(crate) const E_NOT_MULTISIG_MEMBER: u8 = 4;
 
-	#[pallet::pallet]
-	pub struct Pallet<T>(_);
-
 	#[pallet::config]
 	pub trait Config:
 		frame_system::Config<
@@ -108,12 +104,11 @@ pub mod pallet {
 		> + pallet_assets::Config<Balance = Balance, AssetId = AssetId>
 		+ pallet_balances::Config<Balance = Balance>
 		+ darwinia_deposit::Config
-		+ darwinia_staking::Config
 	{
 		/// Override the [`frame_system::Config::RuntimeEvent`].
 		type RuntimeEvent: From<Event> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
 
-		/// Weight information for extrinsics in this pallet.
+		/// Weight information for extrinsic in this pallet.
 		type WeightInfo: WeightInfo;
 	}
 
@@ -158,7 +153,7 @@ pub mod pallet {
 	#[pallet::getter(fn deposit_of)]
 	pub type Deposits<T: Config> = StorageMap<_, Blake2_128Concat, AccountId32, Vec<Deposit>>;
 
-	/// [`darwinia_staking::migration::v2::OldLedger`] data.
+	/// Old ledger data.
 	#[pallet::storage]
 	#[pallet::getter(fn ledger_of)]
 	pub type Ledgers<T: Config> = StorageMap<_, Blake2_128Concat, AccountId32, OldLedger>;
@@ -169,6 +164,41 @@ pub mod pallet {
 	#[pallet::getter(fn multisig_of)]
 	pub type Multisigs<T: Config> = StorageMap<_, Identity, AccountId32, MultisigMigrationDetail>;
 
+	#[pallet::pallet]
+	pub struct Pallet<T>(_);
+	#[pallet::hooks]
+	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+		fn on_idle(_: BlockNumberFor<T>, mut remaining_weight: Weight) -> Weight {
+			const MAX_TASKS: usize = 10;
+
+			#[cfg(test)]
+			let wt = Weight::zero().add_ref_time(1);
+			#[cfg(not(test))]
+			let wt = T::DbWeight::get().writes(1);
+			let mut consumer = <Ledgers<T>>::iter().drain();
+
+			for i in 0..MAX_TASKS {
+				if i >= MAX_TASKS {
+					break;
+				}
+
+				if let Some(rw) = remaining_weight.checked_sub(&wt) {
+					remaining_weight = rw;
+				} else {
+					break;
+				}
+
+				if consumer.next().is_none() {
+					// There is nothing to do; add the weight back.
+					remaining_weight += wt;
+
+					break;
+				}
+			}
+
+			remaining_weight
+		}
+	}
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		/// Migrate all the account data under the `from` to `to`.
@@ -314,7 +344,7 @@ pub mod pallet {
 						if who == submitter {
 							// Reject duplicative submission.
 							if *ok {
-								return InvalidTransaction::Custom(E_DUPLICATIVE_SUBMISSION).into();
+								return InvalidTransaction::Custom(E_DUPLICATED_SUBMISSION).into();
 							}
 
 							is_member = true;
@@ -353,7 +383,7 @@ pub mod pallet {
 
 		fn pre_check_duplicative(multisig: &AccountId32) -> Result<(), TransactionValidityError> {
 			if <Multisigs<T>>::contains_key(multisig) {
-				Err(InvalidTransaction::Custom(E_DUPLICATIVE_SUBMISSION))?
+				Err(InvalidTransaction::Custom(E_DUPLICATED_SUBMISSION))?
 			} else {
 				Ok(())
 			}
@@ -417,32 +447,16 @@ pub mod pallet {
 						asset_details,
 					);
 				}
-				if let Some(l) = <Ledgers<T>>::take(from) {
-					if l.staked_ring > 0 {
-						<pallet_balances::Pallet<T> as Currency<_>>::transfer(
-							to,
-							&darwinia_staking::account_id(),
-							l.staked_ring,
-							AllowDeath,
-						)?;
-					}
-
-					if let Some(ds) = <Deposits<T>>::take(from) {
-						<pallet_balances::Pallet<T> as Currency<_>>::transfer(
-							to,
-							&darwinia_deposit::account_id(),
-							ds.iter().map(|d| d.value).sum(),
-							AllowDeath,
-						)?;
-						<darwinia_deposit::Deposits<T>>::insert(
-							to,
-							BoundedVec::try_from(ds).map_err(|_| <Error<T>>::ExceedMaxDeposits)?,
-						);
-					}
-
-					<darwinia_staking::Ledgers<T>>::insert(
+				if let Some(ds) = <Deposits<T>>::take(from) {
+					<pallet_balances::Pallet<T> as Currency<_>>::transfer(
+						to,
+						&darwinia_deposit::account_id(),
+						ds.iter().map(|d| d.value).sum(),
+						AllowDeath,
+					)?;
+					<darwinia_deposit::Deposits<T>>::insert(
 						to,
-						Ledger { ring: l.staked_ring, deposits: l.staked_deposits },
+						BoundedVec::try_from(ds).map_err(|_| <Error<T>>::ExceedMaxDeposits)?,
 					);
 				}
 			}

pallet/account-migration/src/mock.rs

@@ -106,17 +106,6 @@ impl darwinia_deposit::Config for Runtime {
 	type WeightInfo = ();
 }
 
-impl darwinia_staking::Config for Runtime {
-	type Currency = Balances;
-	type IssuingManager = ();
-	type KtonStaking = ();
-	type RingStaking = ();
-	type RuntimeEvent = RuntimeEvent;
-	type Treasury = ();
-	type UnixTime = Timestamp;
-	type WeightInfo = ();
-}
-
 impl crate::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
 	type WeightInfo = ();
@@ -129,7 +118,6 @@ frame_support::construct_runtime! {
 		Balances: pallet_balances,
 		Assets: pallet_assets,
 		Deposit: darwinia_deposit,
-		Staking: darwinia_staking,
 		AccountMigration: crate,
 	}
 }

pallet/account-migration/src/tests.rs

@@ -19,9 +19,10 @@
 // darwinia
 use crate::{mock::*, *};
 // polkadot-sdk
-use frame_support::{assert_noop, assert_ok};
+use frame_support::{assert_noop, assert_ok, traits::OnIdle};
 use frame_system::AccountInfo;
 use pallet_balances::AccountData;
+use sp_core::H256;
 use sp_keyring::{ed25519::Keyring as Ek, sr25519::Keyring as Sk};
 use sp_runtime::{
 	traits::ValidateUnsigned,
@@ -139,7 +140,7 @@ fn migrate_multisig_should_work() {
 				signature: signature.0,
 				new_multisig_params: None
 			}),
-			TransactionValidityError::Invalid(InvalidTransaction::Custom(E_DUPLICATIVE_SUBMISSION))
+			TransactionValidityError::Invalid(InvalidTransaction::Custom(E_DUPLICATED_SUBMISSION))
 		);
 
 		assert!(<Multisigs<Runtime>>::get(&multisig).is_some());
@@ -176,3 +177,22 @@ fn migrate_multisig_should_work() {
 		assert_eq!(<frame_system::Account<Runtime>>::get(to).consumers, 1);
 	});
 }
+
+#[test]
+fn on_idle_should_work() {
+	new_test_ext().execute_with(|| {
+		(0..1_024).for_each(|i| {
+			<Ledgers<Runtime>>::insert(
+				AccountId32::from(H256::from_low_u64_le(i).0),
+				OldLedger::default(),
+			);
+		});
+		assert_eq!(<Ledgers<Runtime>>::iter().count(), 1_024);
+
+		<AccountMigration as OnIdle<_>>::on_idle(0, Weight::zero().add_ref_time(5));
+		assert_eq!(<Ledgers<Runtime>>::iter().count(), 1_019);
+
+		<AccountMigration as OnIdle<_>>::on_idle(0, Weight::MAX);
+		assert_eq!(<Ledgers<Runtime>>::iter().count(), 1_009);
+	});
+}

pallet/deposit/src/lib.rs

@@ -55,7 +55,7 @@ use frame_support::{
 use frame_system::pallet_prelude::*;
 use sp_core::H160;
 use sp_runtime::traits::AccountIdConversion;
-use sp_std::{collections::btree_map::BTreeMap, prelude::*};
+use sp_std::prelude::*;
 
 #[frame_support::pallet]
 pub mod pallet {
@@ -119,52 +119,32 @@ pub mod pallet {
 	#[pallet::hooks]
 	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
 		fn on_idle(_: BlockNumberFor<T>, mut remaining_weight: Weight) -> Weight {
-			// At least 1 read weight is required.
-			#[cfg(not(test))]
-			if let Some(rw) = remaining_weight.checked_sub(&T::DbWeight::get().reads(1)) {
-				remaining_weight = rw;
-			} else {
-				return remaining_weight;
-			}
-
 			#[cfg(test)]
 			let wt = Weight::zero().add_ref_time(10);
 			#[cfg(not(test))]
 			let wt = <T as Config>::WeightInfo::migrate_for();
-			let mut ds_to_migrate = BTreeMap::<T::AccountId, (Vec<Deposit>, usize)>::new();
-
-			'outer: for (w, ds) in <Deposits<T>>::iter() {
-				for _ in 0..ds.len().div_ceil(10) {
-					if let Some(rw) = remaining_weight.checked_sub(&wt) {
-						remaining_weight = rw;
-
-						if let Some((_, cnt)) = ds_to_migrate.get_mut(&w) {
-							*cnt += 1;
-						} else {
-							ds_to_migrate.insert(w.clone(), (ds.to_vec(), 1));
-						}
-					} else {
-						break 'outer;
-					}
-				}
+
+			if let Some(rw) = remaining_weight.checked_sub(&wt) {
+				remaining_weight = rw;
+			} else {
+				return remaining_weight;
 			}
 
-			for (w, (ds, cnt)) in ds_to_migrate {
-				let mut ds = ds;
+			let Some((k, v)) = <Deposits<T>>::iter().drain().next() else {
+				// There is nothing to do; add the weight back.
+				remaining_weight += wt;
 
-				for _ in 0..cnt {
-					match Self::migrate_for_inner(&w, ds.clone()) {
-						Ok(ds_) => ds = ds_,
-						_ => break,
-					}
-				}
+				return remaining_weight;
+			};
 
-				if ds.is_empty() {
-					<Deposits<T>>::remove(&w);
-				} else {
+			if let Ok(remaining_deposits) = Self::migrate_for_inner(&k, v.clone()) {
+				if !remaining_deposits.is_empty() {
 					// There are still some deposits left for this account.
-					<Deposits<T>>::insert(&w, BoundedVec::truncate_from(ds));
+					<Deposits<T>>::insert(&k, remaining_deposits);
 				}
+			} else {
+				// Put the deposits back if migration failed.
+				<Deposits<T>>::insert(&k, v);
 			}
 
 			remaining_weight
@@ -178,12 +158,12 @@ pub mod pallet {
 		pub fn migrate_for(origin: OriginFor<T>, who: T::AccountId) -> DispatchResult {
 			ensure_signed(origin)?;
 
-			let ds = <Deposits<T>>::take(&who).ok_or(<Error<T>>::NoDeposit)?;
-			let ds = Self::migrate_for_inner(&who, ds)?;
+			let deposits = <Deposits<T>>::take(&who).ok_or(<Error<T>>::NoDeposit)?;
+			let deposits = Self::migrate_for_inner(&who, deposits)?;
 
 			// Put the rest deposits back.
-			if !ds.is_empty() {
-				<Deposits<T>>::insert(&who, BoundedVec::truncate_from(ds));
+			if !deposits.is_empty() {
+				<Deposits<T>>::insert(&who, deposits);
 			}
 
 			Ok(())
@@ -214,7 +194,7 @@ pub mod pallet {
 		fn migrate_for_inner<I>(
 			who: &T::AccountId,
 			deposits: I,
-		) -> Result<Vec<Deposit>, DispatchError>
+		) -> Result<BoundedVec<Deposit, ConstU32<512>>, DispatchError>
 		where
 			I: IntoIterator<Item = Deposit>,
 		{
@@ -251,7 +231,7 @@ pub mod pallet {
 				});
 			}
 
-			Ok(deposits.collect())
+			Ok(BoundedVec::truncate_from(deposits.collect()))
 		}
 	}
 }