fixed migrations

Signed-off-by: muraca <[email protected]>

substrate/frame/assets/src/migration/v1.rs

@@ -15,48 +15,57 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::{migration::v2::V1AssetDetails, *};
-use frame_support::traits::OnRuntimeUpgrade;
+use crate::{
+	migration::v2::old::{Asset, AssetDetails},
+	AssetStatus, Config, DepositBalanceOf, Pallet, LOG_TARGET,
+};
+use frame_support::{
+	pallet_prelude::*, sp_runtime::traits::Saturating, traits::OnRuntimeUpgrade, weights::Weight,
+};
 use log;
 
 #[cfg(feature = "try-runtime")]
 use sp_runtime::TryRuntimeError;
 
-use frame_support::{pallet_prelude::*, weights::Weight};
+mod old {
+	use super::*;
 
-#[derive(Decode)]
-pub struct V0AssetDetails<Balance, AccountId, DepositBalance> {
-	pub owner: AccountId,
-	pub issuer: AccountId,
-	pub admin: AccountId,
-	pub freezer: AccountId,
-	pub supply: Balance,
-	pub deposit: DepositBalance,
-	pub min_balance: Balance,
-	pub is_sufficient: bool,
-	pub accounts: u32,
-	pub sufficients: u32,
-	pub approvals: u32,
-	pub is_frozen: bool,
-}
+	#[derive(Decode)]
+	pub struct AssetDetails<Balance, AccountId, DepositBalance> {
+		pub owner: AccountId,
+		pub issuer: AccountId,
+		pub admin: AccountId,
+		pub freezer: AccountId,
+		pub supply: Balance,
+		pub deposit: DepositBalance,
+		pub min_balance: Balance,
+		pub is_sufficient: bool,
+		pub accounts: u32,
+		pub sufficients: u32,
+		pub approvals: u32,
+		pub is_frozen: bool,
+	}
 
-impl<Balance, AccountId, DepositBalance> V0AssetDetails<Balance, AccountId, DepositBalance> {
-	fn migrate_to_v1(self) -> V1AssetDetails<Balance, AccountId, DepositBalance> {
-		let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live };
+	impl<Balance, AccountId, DepositBalance> AssetDetails<Balance, AccountId, DepositBalance> {
+		pub(super) fn migrate_to_v1(
+			self,
+		) -> super::AssetDetails<Balance, AccountId, DepositBalance> {
+			let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live };
 
-		V1AssetDetails {
-			owner: self.owner,
-			issuer: self.issuer,
-			admin: self.admin,
-			freezer: self.freezer,
-			supply: self.supply,
-			deposit: self.deposit,
-			min_balance: self.min_balance,
-			is_sufficient: self.is_sufficient,
-			accounts: self.accounts,
-			sufficients: self.sufficients,
-			approvals: self.approvals,
-			status,
+			super::AssetDetails {
+				owner: self.owner,
+				issuer: self.issuer,
+				admin: self.admin,
+				freezer: self.freezer,
+				supply: self.supply,
+				deposit: self.deposit,
+				min_balance: self.min_balance,
+				is_sufficient: self.is_sufficient,
+				accounts: self.accounts,
+				sufficients: self.sufficients,
+				approvals: self.approvals,
+				status,
+			}
 		}
 	}
 }
@@ -68,14 +77,13 @@ impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
 		let onchain_version = Pallet::<T>::on_chain_storage_version();
 		if onchain_version == 0 && current_version == 1 {
 			let mut translated = 0u64;
-			/* TODO: fix this
-			Asset::<T>::translate::<V0AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T>>, _>(
-				|_key, old_value| {
-					translated.saturating_inc();
-					Some(old_value.migrate_to_v1())
-				},
-			);
-			 */
+			Asset::<T, ()>::translate::<
+				old::AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T>>,
+				_,
+			>(|_key, old_value| {
+				translated.saturating_inc();
+				Some(old_value.migrate_to_v1())
+			});
 			current_version.put::<Pallet<T>>();
 			log::info!(
 				target: LOG_TARGET,
@@ -99,15 +107,15 @@ impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
 			Pallet::<T>::on_chain_storage_version() == 0,
 			"must upgrade linearly"
 		);
-		let prev_count = Asset::<T>::iter().count();
+		let prev_count = Asset::<T, ()>::iter().count();
 		Ok((prev_count as u32).encode())
 	}
 
 	#[cfg(feature = "try-runtime")]
 	fn post_upgrade(prev_count: Vec<u8>) -> Result<(), TryRuntimeError> {
 		let prev_count: u32 = Decode::decode(&mut prev_count.as_slice())
 			.expect("the state parameter should be something that was generated by pre_upgrade");
-		let post_count = Asset::<T>::iter().count() as u32;
+		let post_count = Asset::<T, ()>::iter().count() as u32;
 		ensure!(
 			prev_count == post_count,
 			"the asset count before and after the migration should be the same"
@@ -122,7 +130,7 @@ impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
 			"after migration, the current_version and onchain_version should be the same"
 		);
 
-		Asset::<T>::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> {
+		Asset::<T, ()>::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> {
 			ensure!(
 					asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen,
 					 "assets should only be live or frozen. None should be in destroying status, or undefined state"

substrate/frame/assets/src/migration/v2.rs

@@ -15,50 +15,72 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::*;
-use frame_support::traits::OnRuntimeUpgrade;
+use crate::{Asset, AssetDetails, AssetStatus, Config, DepositBalanceOf, Pallet};
+use frame_support::{
+	pallet_prelude::*,
+	sp_runtime::traits::{Saturating, Zero},
+	traits::OnRuntimeUpgrade,
+	weights::Weight,
+};
 
 #[cfg(feature = "try-runtime")]
 use sp_runtime::TryRuntimeError;
 
-use frame_support::{pallet_prelude::*, weights::Weight};
+pub mod old {
+	use super::*;
 
-#[derive(Decode)]
-pub struct V1AssetDetails<Balance, AccountId, DepositBalance> {
-	pub owner: AccountId,
-	pub issuer: AccountId,
-	pub admin: AccountId,
-	pub freezer: AccountId,
-	pub supply: Balance,
-	pub deposit: DepositBalance,
-	pub min_balance: Balance,
-	pub is_sufficient: bool,
-	pub accounts: u32,
-	pub sufficients: u32,
-	pub approvals: u32,
-	pub status: AssetStatus,
-}
+	#[frame_support::storage_alias]
+	/// Details of an asset.
+	pub type Asset<T: Config<I>, I: 'static> = StorageMap<
+		Pallet<T, I>,
+		Blake2_128Concat,
+		<T as Config<I>>::AssetId,
+		AssetDetails<
+			<T as Config<I>>::Balance,
+			<T as frame_system::Config>::AccountId,
+			DepositBalanceOf<T, I>,
+		>,
+	>;
+
+	#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
+	pub(crate) struct AssetDetails<Balance, AccountId, DepositBalance> {
+		pub owner: AccountId,
+		pub issuer: AccountId,
+		pub admin: AccountId,
+		pub freezer: AccountId,
+		pub supply: Balance,
+		pub deposit: DepositBalance,
+		pub min_balance: Balance,
+		pub is_sufficient: bool,
+		pub accounts: u32,
+		pub sufficients: u32,
+		pub approvals: u32,
+		pub status: AssetStatus,
+	}
 
-impl<Balance, AccountId, DepositBalance> V1AssetDetails<Balance, AccountId, DepositBalance> {
-	fn migrate_to_v2(self, inactive: Balance) -> AssetDetails<Balance, AccountId, DepositBalance> {
-		AssetDetails {
-			owner: self.owner,
-			issuer: self.issuer,
-			admin: self.admin,
-			freezer: self.freezer,
-			supply: self.supply,
-			inactive,
-			deposit: self.deposit,
-			min_balance: self.min_balance,
-			is_sufficient: self.is_sufficient,
-			accounts: self.accounts,
-			sufficients: self.sufficients,
-			approvals: self.approvals,
-			status: self.status,
+	impl<Balance, AccountId, DepositBalance> AssetDetails<Balance, AccountId, DepositBalance> {
+		pub(super) fn migrate_to_v2(
+			self,
+			inactive: Balance,
+		) -> super::AssetDetails<Balance, AccountId, DepositBalance> {
+			super::AssetDetails {
+				owner: self.owner,
+				issuer: self.issuer,
+				admin: self.admin,
+				freezer: self.freezer,
+				supply: self.supply,
+				inactive,
+				deposit: self.deposit,
+				min_balance: self.min_balance,
+				is_sufficient: self.is_sufficient,
+				accounts: self.accounts,
+				sufficients: self.sufficients,
+				approvals: self.approvals,
+				status: self.status,
+			}
 		}
 	}
 }
-
 /// This migration moves all the state to v2 of Assets
 pub struct VersionUncheckedMigrateToV2<T: Config<I>, I: 'static, A>(
 	sp_std::marker::PhantomData<(T, I, A)>,
@@ -68,7 +90,7 @@ impl<T: Config<I>, I: 'static, A> OnRuntimeUpgrade for VersionUncheckedMigrateTo
 	fn on_runtime_upgrade() -> Weight {
 		let mut translated = 0u64;
 		Asset::<T, I>::translate::<
-			V1AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T, I>>,
+			old::AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T, I>>,
 			_,
 		>(|_asset_id, old_value| {
 			translated.saturating_inc();
@@ -83,7 +105,7 @@ impl<T: Config<I>, I: 'static, A> OnRuntimeUpgrade for VersionUncheckedMigrateTo
 	#[cfg(feature = "try-runtime")]
 	fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
 		log::info!("pre-migration assets v2");
-		let prev_count = Asset::<T, I>::iter().count();
+		let prev_count = old::Asset::<T, I>::iter().count();
 		Ok((prev_count as u32).encode())
 	}
 

substrate/frame/assets/src/tests.rs

@@ -22,6 +22,7 @@ use crate::{mock::*, Error};
 use frame_support::{
 	assert_noop, assert_ok,
 	dispatch::GetDispatchInfo,
+	pallet_prelude::StorageVersion,
 	traits::{fungibles::InspectEnumerable, tokens::Preservation::Protect, Currency},
 };
 use pallet_balances::Error as BalancesError;
@@ -1775,3 +1776,65 @@ fn asset_destroy_refund_existence_deposit() {
 		assert_eq!(Balances::reserved_balance(&admin), 0);
 	});
 }
+
+#[test]
+fn migrate_to_v2_works() {
+	new_test_ext().execute_with(|| {
+		use crate::migration::v2::old as v1;
+		use frame_support::traits::OnRuntimeUpgrade;
+		StorageVersion::new(1).put::<Pallet<Test, ()>>();
+
+		v1::Asset::<Test, ()>::insert(
+			1,
+			v1::AssetDetails::<
+				<Test as Config>::Balance,
+				<Test as frame_system::Config>::AccountId,
+				DepositBalanceOf<Test>,
+			> {
+				owner: 1,
+				issuer: 1,
+				admin: 1,
+				freezer: 1,
+				supply: 102,
+				deposit: 0,
+				min_balance: 1,
+				is_sufficient: false,
+				accounts: 0,
+				sufficients: 0,
+				approvals: 0,
+				status: AssetStatus::Live,
+			},
+		);
+
+		Account::<Test, ()>::insert(
+			1,
+			1,
+			AssetAccountOf::<Test, ()> {
+				balance: 100,
+				status: AccountStatus::Liquid,
+				reason: ExistenceReason::<_, _>::Sufficient,
+				extra: Default::default(),
+			},
+		);
+		Account::<Test, ()>::insert(
+			1,
+			2,
+			AssetAccountOf::<Test, ()> {
+				balance: 2,
+				status: AccountStatus::Liquid,
+				reason: ExistenceReason::<_, _>::Sufficient,
+				extra: Default::default(),
+			},
+		);
+
+		// Run migration.
+		assert_ok!(
+			crate::migration::v2::VersionCheckedMigrateToV2::<Test, (), ()>::try_on_runtime_upgrade(
+				true
+			)
+		);
+
+		// TODO check with 2 after setting account 2's balance as inactive
+		assert_eq!(Asset::<Test, ()>::get(1).unwrap().inactive, 0);
+	});
+}