Improve deposits migration

pallet/deposit/Cargo.toml

@@ -25,7 +25,6 @@ frame-support    = { workspace = true }
 frame-system     = { workspace = true }
 pallet-timestamp = { workspace = true }
 sp-core          = { workspace = true }
-sp-runtime       = { workspace = true }
 sp-std           = { workspace = true }
 # polkadot-sdk optional
 frame-benchmarking = { workspace = true, optional = true }
@@ -34,6 +33,7 @@ frame-benchmarking = { workspace = true, optional = true }
 # polkadot-sdk
 pallet-balances = { workspace = true, features = ["std"] }
 sp-io           = { workspace = true, features = ["std"] }
+sp-runtime      = { workspace = true, features = ["std"] }
 
 [features]
 default = ["std"]

pallet/deposit/src/lib.rs

@@ -40,7 +40,6 @@ pub use weights::WeightInfo;
 // core
 use core::marker::PhantomData;
 // crates.io
-use codec::FullCodec;
 use ethabi::{Function, Param, ParamType, StateMutability, Token};
 // darwinia
 use dc_types::{Balance, Moment};
@@ -50,11 +49,9 @@ use fp_evm::{CallOrCreateInfo, ExitReason};
 use frame_support::{
 	pallet_prelude::*,
 	traits::{Currency, ExistenceRequirement::AllowDeath, UnixTime},
-	PalletId,
 };
 use frame_system::pallet_prelude::*;
 use sp_core::H160;
-use sp_runtime::traits::AccountIdConversion;
 use sp_std::prelude::*;
 
 #[frame_support::pallet]
@@ -204,7 +201,7 @@ pub mod pallet {
 			let mut to_migrate = (0, Vec::new(), Vec::new());
 
 			// Take 0~10 deposits to migrate.
-			for d in deposits.by_ref().take(10) {
+			for d in deposits.by_ref().take(10).filter(|d| d.value != 0) {
 				if d.expired_time <= now {
 					to_claim.0 += d.value;
 					to_claim.1.push(d.id);
@@ -216,14 +213,13 @@ pub mod pallet {
 			}
 
 			if to_claim.0 != 0 {
-				T::Ring::transfer(&account_id(), who, to_claim.0, AllowDeath)?;
+				T::Ring::transfer(&T::Treasury::get(), who, to_claim.0, AllowDeath)?;
 				Self::deposit_event(Event::DepositsClaimed {
 					owner: who.clone(),
 					deposits: to_claim.1,
 				});
 			}
 			if to_migrate.0 != 0 {
-				T::Ring::transfer(&account_id(), &T::Treasury::get(), to_migrate.0, AllowDeath)?;
 				T::DepositMigrator::migrate(who.clone(), to_migrate.0, to_migrate.2)?;
 				Self::deposit_event(Event::DepositsMigrated {
 					owner: who.clone(),
@@ -338,11 +334,3 @@ where
 		}
 	}
 }
-
-/// The account of the deposit pot.
-pub fn account_id<A>() -> A
-where
-	A: FullCodec,
-{
-	PalletId(*b"dar/depo").into_account_truncating()
-}

pallet/deposit/src/mock.rs

@@ -71,3 +71,7 @@ pub fn new_test_ext() -> TestExternalities {
 
 	storage.into()
 }
+
+pub fn events() -> Vec<Event<Runtime>> {
+	System::read_events_for_pallet()
+}

pallet/deposit/src/tests.rs

@@ -27,7 +27,7 @@ use frame_support::{assert_ok, traits::OnIdle};
 #[test]
 fn migrate_should_work() {
 	new_test_ext().execute_with(|| {
-		let _ = Balances::deposit_creating(&account_id(), 2);
+		let _ = Balances::deposit_creating(&0, 2);
 
 		<Deposits<Runtime>>::insert(
 			1,
@@ -52,9 +52,18 @@ fn on_idle_should_work() {
 				.collect(),
 		)
 	}
+	fn mock_zero_deposits(count: u16) -> BoundedVec<DepositS, ConstU32<512>> {
+		BoundedVec::truncate_from(
+			(0..count)
+				.map(|id| DepositS { id, value: 0, start_time: 0, expired_time: 0, in_use: false })
+				.collect(),
+		)
+	}
 
 	new_test_ext().execute_with(|| {
-		let _ = Balances::deposit_creating(&account_id(), 10_000);
+		System::set_block_number(1);
+
+		let _ = Balances::deposit_creating(&0, 10_000);
 
 		<Deposits<Runtime>>::insert(1, mock_deposits(512));
 		<Deposits<Runtime>>::insert(2, mock_deposits(512));
@@ -90,11 +99,29 @@ fn on_idle_should_work() {
 		assert_eq!(Deposit::deposit_of(2).unwrap().len(), 512);
 		assert!(Deposit::deposit_of(3).is_none());
 
+		System::reset_events();
 		(0..54).for_each(|_| {
 			<Deposit as OnIdle<_>>::on_idle(0, Weight::MAX);
 		});
+		assert_eq!(events().into_iter().count(), 54);
 		assert!(Deposit::deposit_of(1).is_none());
 		assert!(Deposit::deposit_of(2).is_none());
 		assert!(Deposit::deposit_of(3).is_none());
 	});
+	new_test_ext().execute_with(|| {
+		System::set_block_number(1);
+
+		let _ = Balances::deposit_creating(&0, 10_000);
+
+		<Deposits<Runtime>>::insert(1, mock_zero_deposits(512));
+		assert_eq!(Deposit::deposit_of(1).unwrap().len(), 512);
+
+		System::reset_events();
+		(0..52).for_each(|_| {
+			<Deposit as OnIdle<_>>::on_idle(0, Weight::MAX);
+		});
+		dbg!(events());
+		assert_eq!(events().into_iter().count(), 0);
+		assert!(Deposit::deposit_of(1).is_none());
+	});
 }

runtime/darwinia/src/migration.rs

@@ -86,5 +86,11 @@ fn migrate() -> frame_support::weights::Weight {
 		]),
 	);
 
-	<Runtime as frame_system::Config>::DbWeight::get().reads_writes(r, w + 5)
+	let _ = Balances::transfer_all(
+		RuntimeOrigin::signed(PalletId(*b"dar/depo").into_account_truncating()),
+		&Treasury::account_id(),
+		false,
+	);
+
+	<Runtime as frame_system::Config>::DbWeight::get().reads_writes(r, w + 10)
 }