diff --git a/Cargo.lock b/Cargo.lock index bd45d433e..01fd4d89d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2708,7 +2708,6 @@ version = "6.7.2" dependencies = [ "array-bytes 6.2.3", "darwinia-deposit", - "darwinia-staking", "dc-primitives", "frame-benchmarking", "frame-support", diff --git a/pallet/account-migration/Cargo.toml b/pallet/account-migration/Cargo.toml index 9a8b9aa02..6a8f5896e 100644 --- a/pallet/account-migration/Cargo.toml +++ b/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", diff --git a/pallet/account-migration/src/lib.rs b/pallet/account-migration/src/lib.rs index b9991561a..a4810d202 100644 --- a/pallet/account-migration/src/lib.rs +++ b/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(_); - #[pallet::config] pub trait Config: frame_system::Config< @@ -108,12 +104,11 @@ pub mod pallet { > + pallet_assets::Config + pallet_balances::Config + darwinia_deposit::Config - + darwinia_staking::Config { /// Override the [`frame_system::Config::RuntimeEvent`]. type RuntimeEvent: From + IsType<::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 = StorageMap<_, Blake2_128Concat, AccountId32, Vec>; - /// [`darwinia_staking::migration::v2::OldLedger`] data. + /// Old ledger data. #[pallet::storage] #[pallet::getter(fn ledger_of)] pub type Ledgers = StorageMap<_, Blake2_128Concat, AccountId32, OldLedger>; @@ -169,6 +164,41 @@ pub mod pallet { #[pallet::getter(fn multisig_of)] pub type Multisigs = StorageMap<_, Identity, AccountId32, MultisigMigrationDetail>; + #[pallet::pallet] + pub struct Pallet(_); + #[pallet::hooks] + impl Hooks> for Pallet { + fn on_idle(_: BlockNumberFor, 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 = >::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 Pallet { /// 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 >::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) = >::take(from) { - if l.staked_ring > 0 { - as Currency<_>>::transfer( - to, - &darwinia_staking::account_id(), - l.staked_ring, - AllowDeath, - )?; - } - - if let Some(ds) = >::take(from) { - as Currency<_>>::transfer( - to, - &darwinia_deposit::account_id(), - ds.iter().map(|d| d.value).sum(), - AllowDeath, - )?; - >::insert( - to, - BoundedVec::try_from(ds).map_err(|_| >::ExceedMaxDeposits)?, - ); - } - - >::insert( + if let Some(ds) = >::take(from) { + as Currency<_>>::transfer( + to, + &darwinia_deposit::account_id(), + ds.iter().map(|d| d.value).sum(), + AllowDeath, + )?; + >::insert( to, - Ledger { ring: l.staked_ring, deposits: l.staked_deposits }, + BoundedVec::try_from(ds).map_err(|_| >::ExceedMaxDeposits)?, ); } } diff --git a/pallet/account-migration/src/mock.rs b/pallet/account-migration/src/mock.rs index 6a02d8a12..99b3dea39 100644 --- a/pallet/account-migration/src/mock.rs +++ b/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, } } diff --git a/pallet/account-migration/src/tests.rs b/pallet/account-migration/src/tests.rs index be73f4779..5b12496b2 100644 --- a/pallet/account-migration/src/tests.rs +++ b/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!(>::get(&multisig).is_some()); @@ -176,3 +177,22 @@ fn migrate_multisig_should_work() { assert_eq!(>::get(to).consumers, 1); }); } + +#[test] +fn on_idle_should_work() { + new_test_ext().execute_with(|| { + (0..1_024).for_each(|i| { + >::insert( + AccountId32::from(H256::from_low_u64_le(i).0), + OldLedger::default(), + ); + }); + assert_eq!(>::iter().count(), 1_024); + + >::on_idle(0, Weight::zero().add_ref_time(5)); + assert_eq!(>::iter().count(), 1_019); + + >::on_idle(0, Weight::MAX); + assert_eq!(>::iter().count(), 1_009); + }); +} diff --git a/pallet/deposit/src/lib.rs b/pallet/deposit/src/lib.rs index 35c892c82..e3980b825 100644 --- a/pallet/deposit/src/lib.rs +++ b/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 Hooks> for Pallet { fn on_idle(_: BlockNumberFor, 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 = ::WeightInfo::migrate_for(); - let mut ds_to_migrate = BTreeMap::, usize)>::new(); - - 'outer: for (w, ds) in >::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)) = >::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() { - >::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. - >::insert(&w, BoundedVec::truncate_from(ds)); + >::insert(&k, remaining_deposits); } + } else { + // Put the deposits back if migration failed. + >::insert(&k, v); } remaining_weight @@ -178,12 +158,12 @@ pub mod pallet { pub fn migrate_for(origin: OriginFor, who: T::AccountId) -> DispatchResult { ensure_signed(origin)?; - let ds = >::take(&who).ok_or(>::NoDeposit)?; - let ds = Self::migrate_for_inner(&who, ds)?; + let deposits = >::take(&who).ok_or(>::NoDeposit)?; + let deposits = Self::migrate_for_inner(&who, deposits)?; // Put the rest deposits back. - if !ds.is_empty() { - >::insert(&who, BoundedVec::truncate_from(ds)); + if !deposits.is_empty() { + >::insert(&who, deposits); } Ok(()) @@ -214,7 +194,7 @@ pub mod pallet { fn migrate_for_inner( who: &T::AccountId, deposits: I, - ) -> Result, DispatchError> + ) -> Result>, DispatchError> where I: IntoIterator, { @@ -251,7 +231,7 @@ pub mod pallet { }); } - Ok(deposits.collect()) + Ok(BoundedVec::truncate_from(deposits.collect())) } } } diff --git a/pallet/deposit/src/tests.rs b/pallet/deposit/src/tests.rs index deecaf726..f4e1fedf7 100644 --- a/pallet/deposit/src/tests.rs +++ b/pallet/deposit/src/tests.rs @@ -63,35 +63,38 @@ fn on_idle_should_work() { assert_eq!(Deposit::deposit_of(2).unwrap().len(), 512); assert_eq!(Deposit::deposit_of(3).unwrap().len(), 512); - AllPalletsWithSystem::on_idle(0, Weight::zero()); + >::on_idle(0, Weight::zero()); assert_eq!(Deposit::deposit_of(1).unwrap().len(), 512); assert_eq!(Deposit::deposit_of(2).unwrap().len(), 512); assert_eq!(Deposit::deposit_of(3).unwrap().len(), 512); - AllPalletsWithSystem::on_idle(0, Weight::MAX); - assert!(Deposit::deposit_of(1).is_none()); - assert!(Deposit::deposit_of(2).is_none()); - assert!(Deposit::deposit_of(3).is_none()); - - // --- - - >::insert(1, mock_deposits(512)); - >::insert(2, mock_deposits(512)); - >::insert(3, mock_deposits(512)); + >::on_idle(0, Weight::zero().add_ref_time(10)); assert_eq!(Deposit::deposit_of(1).unwrap().len(), 512); assert_eq!(Deposit::deposit_of(2).unwrap().len(), 512); - assert_eq!(Deposit::deposit_of(3).unwrap().len(), 512); + assert_eq!(Deposit::deposit_of(3).unwrap().len(), 502); + >::on_idle(0, Weight::MAX); + assert_eq!(Deposit::deposit_of(1).unwrap().len(), 512); + assert_eq!(Deposit::deposit_of(2).unwrap().len(), 512); + assert_eq!(Deposit::deposit_of(3).unwrap().len(), 492); - AllPalletsWithSystem::on_idle(0, Weight::zero().add_ref_time(256)); + (0..50).for_each(|_| { + >::on_idle(0, Weight::MAX); + }); assert_eq!(Deposit::deposit_of(1).unwrap().len(), 512); assert_eq!(Deposit::deposit_of(2).unwrap().len(), 512); - // 512 - 256 / 10 * 10 = 252 - assert_eq!(Deposit::deposit_of(3).unwrap().len(), 262); - AllPalletsWithSystem::on_idle(0, Weight::zero().add_ref_time(1_234)); + assert!(Deposit::deposit_of(3).is_none()); + + (0..50).for_each(|_| { + >::on_idle(0, Weight::MAX); + }); + assert_eq!(Deposit::deposit_of(1).unwrap().len(), 12); + assert_eq!(Deposit::deposit_of(2).unwrap().len(), 512); + assert!(Deposit::deposit_of(3).is_none()); + + (0..54).for_each(|_| { + >::on_idle(0, Weight::MAX); + }); assert!(Deposit::deposit_of(1).is_none()); - // 512 => 520 weight - // 1_234 - 520 * 2 = 194 - // 262 - 194 / 10 * 10 = 72 - assert_eq!(Deposit::deposit_of(2).unwrap().len(), 72); + assert!(Deposit::deposit_of(2).is_none()); assert!(Deposit::deposit_of(3).is_none()); }); } diff --git a/pallet/staking/src/lib.rs b/pallet/staking/src/lib.rs index d16cc4c9c..667c92450 100644 --- a/pallet/staking/src/lib.rs +++ b/pallet/staking/src/lib.rs @@ -347,64 +347,62 @@ pub mod pallet { } fn idle_allocate_ring_staking_reward(remaining_weight: &mut Weight) { - // At least 1 read weight is required. - if let Some(rw) = remaining_weight.checked_sub(&T::DbWeight::get().reads(1)) { - *remaining_weight = rw; - } else { - return; - } + const MAX_TASKS: usize = 10; #[cfg(test)] - let weight = Weight::zero().add_ref_time(1); + let wt = Weight::zero().add_ref_time(1); #[cfg(not(test))] - let weight = T::WeightInfo::allocate_ring_staking_reward_of(); - let mut reward_to_allocate = Vec::new(); + let wt = T::WeightInfo::allocate_ring_staking_reward_of(); + let mut consumer = >::iter().drain(); - for (who, amount) in >::iter() { - if let Some(rw) = remaining_weight.checked_sub(&weight) { - *remaining_weight = rw; + for i in 0..MAX_TASKS { + if i >= MAX_TASKS { + break; + } - reward_to_allocate.push((who, amount)); + if let Some(rw) = remaining_weight.checked_sub(&wt) { + *remaining_weight = rw; } else { break; } - } - - for (who, amount) in reward_to_allocate { - let _ = Self::allocate_ring_staking_reward_of_inner(who.clone(), amount); + if let Some((k, v)) = consumer.next() { + let _ = Self::allocate_ring_staking_reward_of_inner(k, v); + } else { + // There is nothing to do; add the weight back. + *remaining_weight += wt; - >::remove(&who); + break; + } } } fn idle_unstake(remaining_weight: &mut Weight) { - // At least 1 read weight is required. - if let Some(rw) = remaining_weight.checked_sub(&T::DbWeight::get().reads(1)) { - *remaining_weight = rw; - } else { - return; - } + const MAX_TASKS: usize = 10; #[cfg(test)] - let weight = Weight::zero().add_ref_time(1); + let wt = Weight::zero().add_ref_time(1); #[cfg(not(test))] - let weight = T::WeightInfo::unstake_all_for(); - let mut ledgers_to_migrate = Vec::new(); + let wt = T::WeightInfo::unstake_all_for(); + let mut consumer = >::iter().drain(); - for (who, l) in >::iter() { - if let Some(rw) = remaining_weight.checked_sub(&weight) { - *remaining_weight = rw; + for i in 0..MAX_TASKS { + if i >= MAX_TASKS { + break; + } - ledgers_to_migrate.push((who, l)); + if let Some(rw) = remaining_weight.checked_sub(&wt) { + *remaining_weight = rw; } else { break; } - } - - for (who, l) in ledgers_to_migrate { - let _ = Self::unstake_all_for_inner(who.clone(), l); + if let Some((k, v)) = consumer.next() { + let _ = Self::unstake_all_for_inner(k, v); + } else { + // There is nothing to do; add the weight back. + *remaining_weight += wt; - >::remove(&who); + break; + } } } diff --git a/pallet/staking/src/tests.rs b/pallet/staking/src/tests.rs index ab75b3bfd..c14878ada 100644 --- a/pallet/staking/src/tests.rs +++ b/pallet/staking/src/tests.rs @@ -94,17 +94,26 @@ fn on_idle_allocate_ring_staking_reward_should_work() { (1..=512).for_each(|i| >::insert(AccountId(i), 1)); System::reset_events(); - AllPalletsWithSystem::on_idle(0, Weight::zero().add_ref_time(128)); + >::on_idle(0, Weight::zero().add_ref_time(5)); assert_eq!( events().into_iter().filter(|e| matches!(e, Event::RewardAllocated { .. })).count(), - 128 + 5 ); System::reset_events(); - AllPalletsWithSystem::on_idle(0, Weight::MAX); + >::on_idle(0, Weight::MAX); assert_eq!( events().into_iter().filter(|e| matches!(e, Event::RewardAllocated { .. })).count(), - 384 + 10 + ); + + System::reset_events(); + while >::iter().count() != 0 { + >::on_idle(0, Weight::MAX); + } + assert_eq!( + events().into_iter().filter(|e| matches!(e, Event::RewardAllocated { .. })).count(), + 497 ); }); } @@ -112,6 +121,8 @@ fn on_idle_allocate_ring_staking_reward_should_work() { #[test] fn on_idle_unstake_should_work() { ExtBuilder.build().execute_with(|| { + // Skip 1 to 3 collators. + // Since they have session rewards which make calculation more complex. (4..=515).for_each(|i| { >::insert( AccountId(i), @@ -120,15 +131,17 @@ fn on_idle_unstake_should_work() { }); System::reset_events(); - AllPalletsWithSystem::on_idle(0, Weight::zero().add_ref_time(128)); - assert_eq!(>::iter().count(), 384); + >::on_idle(0, Weight::zero().add_ref_time(5)); + assert_eq!(>::iter().count(), 507); System::reset_events(); - AllPalletsWithSystem::on_idle(0, Weight::MAX); - assert_eq!(>::iter().count(), 0); + >::on_idle(0, Weight::MAX); + assert_eq!(>::iter().count(), 497); + + while >::iter().count() != 0 { + >::on_idle(0, Weight::MAX); + } - // Skip 1 to 3 collators. - // Since they have session rewards which make calculation more complex. (4..515).for_each(|who| { assert_eq!(Balances::free_balance(AccountId(who)), 100 + who as Balance); });