Make Assets Pallet's Privileged Roles Option

frame/assets/Cargo.toml

@@ -23,6 +23,7 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../su
 # `system` module provides us with all sorts of useful stuff and macros depend on it being around.
 frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" }
 frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true }
+log = { version = "0.4.17", default-features = false }
 
 [dev-dependencies]
 sp-core = { version = "7.0.0", path = "../../primitives/core" }

frame/assets/src/functions.rs

@@ -352,7 +352,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	) -> DispatchResult {
 		Self::increase_balance(id, beneficiary, amount, |details| -> DispatchResult {
 			if let Some(check_issuer) = maybe_check_issuer {
-				ensure!(check_issuer == details.issuer, Error::<T, I>::NoPermission);
+				ensure!(Some(check_issuer) == details.issuer, Error::<T, I>::NoPermission);
 			}
 			debug_assert!(
 				T::Balance::max_value() - details.supply >= amount,
@@ -439,7 +439,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let actual = Self::decrease_balance(id, target, amount, f, |actual, details| {
 			// Check admin rights.
 			if let Some(check_admin) = maybe_check_admin {
-				ensure!(check_admin == details.admin, Error::<T, I>::NoPermission);
+				ensure!(Some(check_admin) == details.admin, Error::<T, I>::NoPermission);
 			}
 
 			debug_assert!(details.supply >= actual, "checked in prep; qed");
@@ -564,7 +564,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 			// Check admin rights.
 			if let Some(need_admin) = maybe_need_admin {
-				ensure!(need_admin == details.admin, Error::<T, I>::NoPermission);
+				ensure!(Some(need_admin) == details.admin, Error::<T, I>::NoPermission);
 			}
 
 			// Skip if source == dest
@@ -650,10 +650,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Asset::<T, I>::insert(
 			id,
 			AssetDetails {
-				owner: owner.clone(),
-				issuer: owner.clone(),
-				admin: owner.clone(),
-				freezer: owner.clone(),
+				owner: Some(owner.clone()),
+				issuer: Some(owner.clone()),
+				admin: Some(owner.clone()),
+				freezer: Some(owner.clone()),
 				supply: Zero::zero(),
 				deposit: Zero::zero(),
 				min_balance,
@@ -677,7 +677,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Asset::<T, I>::try_mutate_exists(id, |maybe_details| -> Result<(), DispatchError> {
 			let mut details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
 			if let Some(check_owner) = maybe_check_owner {
-				ensure!(details.owner == check_owner, Error::<T, I>::NoPermission);
+				ensure!(details.owner.clone().unwrap() == check_owner, Error::<T, I>::NoPermission);
 			}
 			details.status = AssetStatus::Destroying;
 
@@ -770,10 +770,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			ensure!(details.approvals == 0, Error::<T, I>::InUse);
 
 			let metadata = Metadata::<T, I>::take(&id);
-			T::Currency::unreserve(
-				&details.owner,
-				details.deposit.saturating_add(metadata.deposit),
-			);
+			if details.owner.is_some() {
+				T::Currency::unreserve(
+					&details.owner.unwrap(),
+					details.deposit.saturating_add(metadata.deposit),
+				);
+			}
 			Self::deposit_event(Event::Destroyed { asset_id: id });
 
 			Ok(())
@@ -891,7 +893,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
 		ensure!(d.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
-		ensure!(from == &d.owner, Error::<T, I>::NoPermission);
+		ensure!(from == &d.owner.unwrap(), Error::<T, I>::NoPermission);
 
 		Metadata::<T, I>::try_mutate_exists(id, |metadata| {
 			ensure!(metadata.as_ref().map_or(true, |m| !m.is_frozen), Error::<T, I>::NoPermission);

frame/assets/src/impl_fungibles.rs

@@ -270,19 +270,19 @@ impl<T: Config<I>, I: 'static> fungibles::roles::Inspect<<T as SystemConfig>::Ac
 	for Pallet<T, I>
 {
 	fn owner(asset: T::AssetId) -> Option<<T as SystemConfig>::AccountId> {
-		Asset::<T, I>::get(asset).map(|x| x.owner)
+		Asset::<T, I>::get(asset).and_then(|x| x.owner)
 	}
 
 	fn issuer(asset: T::AssetId) -> Option<<T as SystemConfig>::AccountId> {
-		Asset::<T, I>::get(asset).map(|x| x.issuer)
+		Asset::<T, I>::get(asset).and_then(|x| x.issuer)
 	}
 
 	fn admin(asset: T::AssetId) -> Option<<T as SystemConfig>::AccountId> {
-		Asset::<T, I>::get(asset).map(|x| x.admin)
+		Asset::<T, I>::get(asset).and_then(|x| x.admin)
 	}
 
 	fn freezer(asset: T::AssetId) -> Option<<T as SystemConfig>::AccountId> {
-		Asset::<T, I>::get(asset).map(|x| x.freezer)
+		Asset::<T, I>::get(asset).and_then(|x| x.freezer)
 	}
 }
 

frame/assets/src/lib.rs

@@ -345,10 +345,10 @@ pub mod pallet {
 				Asset::<T, I>::insert(
 					id,
 					AssetDetails {
-						owner: owner.clone(),
-						issuer: owner.clone(),
-						admin: owner.clone(),
-						freezer: owner.clone(),
+						owner: Some(owner.clone()),
+						issuer: Some(owner.clone()),
+						admin: Some(owner.clone()),
+						freezer: Some(owner.clone()),
 						supply: Zero::zero(),
 						deposit: Zero::zero(),
 						min_balance: *min_balance,
@@ -562,10 +562,10 @@ pub mod pallet {
 			Asset::<T, I>::insert(
 				id,
 				AssetDetails {
-					owner: owner.clone(),
-					issuer: admin.clone(),
-					admin: admin.clone(),
-					freezer: admin.clone(),
+					owner: Some(owner.clone()),
+					issuer: Some(admin.clone()),
+					admin: Some(admin.clone()),
+					freezer: Some(admin.clone()),
 					supply: Zero::zero(),
 					deposit,
 					min_balance,
@@ -874,7 +874,7 @@ pub mod pallet {
 				d.status == AssetStatus::Live || d.status == AssetStatus::Frozen,
 				Error::<T, I>::AssetNotLive
 			);
-			ensure!(origin == d.freezer, Error::<T, I>::NoPermission);
+			ensure!(origin == d.freezer.unwrap(), Error::<T, I>::NoPermission);
 			let who = T::Lookup::lookup(who)?;
 
 			Account::<T, I>::try_mutate(id, &who, |maybe_account| -> DispatchResult {
@@ -909,7 +909,7 @@ pub mod pallet {
 				details.status == AssetStatus::Live || details.status == AssetStatus::Frozen,
 				Error::<T, I>::AssetNotLive
 			);
-			ensure!(origin == details.admin, Error::<T, I>::NoPermission);
+			ensure!(origin == details.admin.unwrap(), Error::<T, I>::NoPermission);
 			let who = T::Lookup::lookup(who)?;
 
 			Account::<T, I>::try_mutate(id, &who, |maybe_account| -> DispatchResult {
@@ -940,7 +940,7 @@ pub mod pallet {
 			Asset::<T, I>::try_mutate(id, |maybe_details| {
 				let d = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
 				ensure!(d.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
-				ensure!(origin == d.freezer, Error::<T, I>::NoPermission);
+				ensure!(origin == d.freezer.clone().unwrap(), Error::<T, I>::NoPermission);
 
 				d.status = AssetStatus::Frozen;
 
@@ -967,7 +967,7 @@ pub mod pallet {
 
 			Asset::<T, I>::try_mutate(id, |maybe_details| {
 				let d = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
-				ensure!(origin == d.admin, Error::<T, I>::NoPermission);
+				ensure!(origin == d.admin.clone().unwrap(), Error::<T, I>::NoPermission);
 				ensure!(d.status == AssetStatus::Frozen, Error::<T, I>::NotFrozen);
 
 				d.status = AssetStatus::Live;
@@ -999,18 +999,25 @@ pub mod pallet {
 			Asset::<T, I>::try_mutate(id, |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
 				ensure!(details.status == AssetStatus::Live, Error::<T, I>::LiveAsset);
-				ensure!(origin == details.owner, Error::<T, I>::NoPermission);
-				if details.owner == owner {
+				ensure!(origin == details.owner.clone().unwrap(), Error::<T, I>::NoPermission);
+				if details.owner == Some(owner.clone()) {
 					return Ok(())
 				}
 
 				let metadata_deposit = Metadata::<T, I>::get(id).deposit;
 				let deposit = details.deposit + metadata_deposit;
 
 				// Move the deposit to the new owner.
-				T::Currency::repatriate_reserved(&details.owner, &owner, deposit, Reserved)?;
+				if details.owner.is_some() {
+					T::Currency::repatriate_reserved(
+						&details.owner.as_ref().unwrap(),
+						&owner,
+						deposit,
+						Reserved,
+					)?;
+				}
 
-				details.owner = owner.clone();
+				details.owner = Some(owner.clone());
 
 				Self::deposit_event(Event::OwnerChanged { asset_id: id, owner });
 				Ok(())
@@ -1045,11 +1052,11 @@ pub mod pallet {
 			Asset::<T, I>::try_mutate(id, |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
 				ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
-				ensure!(origin == details.owner, Error::<T, I>::NoPermission);
+				ensure!(origin == details.owner.clone().unwrap(), Error::<T, I>::NoPermission);
 
-				details.issuer = issuer.clone();
-				details.admin = admin.clone();
-				details.freezer = freezer.clone();
+				details.issuer = Some(issuer.clone());
+				details.admin = Some(admin.clone());
+				details.freezer = Some(freezer.clone());
 
 				Self::deposit_event(Event::TeamChanged { asset_id: id, issuer, admin, freezer });
 				Ok(())
@@ -1104,11 +1111,14 @@ pub mod pallet {
 
 			let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
 			ensure!(d.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
-			ensure!(origin == d.owner, Error::<T, I>::NoPermission);
+			ensure!(origin == d.owner.clone().unwrap(), Error::<T, I>::NoPermission);
 
 			Metadata::<T, I>::try_mutate_exists(id, |metadata| {
 				let deposit = metadata.take().ok_or(Error::<T, I>::Unknown)?.deposit;
-				T::Currency::unreserve(&d.owner, deposit);
+
+				if d.owner.is_some() {
+					T::Currency::unreserve(&d.owner.unwrap(), deposit);
+				}
 				Self::deposit_event(Event::MetadataCleared { asset_id: id });
 				Ok(())
 			})
@@ -1188,7 +1198,9 @@ pub mod pallet {
 			let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
 			Metadata::<T, I>::try_mutate_exists(id, |metadata| {
 				let deposit = metadata.take().ok_or(Error::<T, I>::Unknown)?.deposit;
-				T::Currency::unreserve(&d.owner, deposit);
+				if d.owner.is_some() {
+					T::Currency::unreserve(&d.owner.unwrap(), deposit);
+				}
 				Self::deposit_event(Event::MetadataCleared { asset_id: id });
 				Ok(())
 			})
@@ -1233,10 +1245,10 @@ pub mod pallet {
 			Asset::<T, I>::try_mutate(id, |maybe_asset| {
 				let mut asset = maybe_asset.take().ok_or(Error::<T, I>::Unknown)?;
 				ensure!(asset.status != AssetStatus::Destroying, Error::<T, I>::AssetNotLive);
-				asset.owner = T::Lookup::lookup(owner)?;
-				asset.issuer = T::Lookup::lookup(issuer)?;
-				asset.admin = T::Lookup::lookup(admin)?;
-				asset.freezer = T::Lookup::lookup(freezer)?;
+				asset.owner = Some(T::Lookup::lookup(owner)?);
+				asset.issuer = Some(T::Lookup::lookup(issuer)?);
+				asset.admin = Some(T::Lookup::lookup(admin)?);
+				asset.freezer = Some(T::Lookup::lookup(freezer)?);
 				asset.min_balance = min_balance;
 				asset.is_sufficient = is_sufficient;
 				if is_frozen {
@@ -1343,7 +1355,7 @@ pub mod pallet {
 				.map(|_| ())
 				.or_else(|origin| -> DispatchResult {
 					let origin = ensure_signed(origin)?;
-					ensure!(origin == d.admin, Error::<T, I>::NoPermission);
+					ensure!(Some(origin) == d.admin, Error::<T, I>::NoPermission);
 					Ok(())
 				})?;
 

frame/assets/src/migration.rs

@@ -44,10 +44,10 @@ pub mod v1 {
 			let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live };
 
 			AssetDetails {
-				owner: self.owner,
-				issuer: self.issuer,
-				admin: self.admin,
-				freezer: self.freezer,
+				owner: Some(self.owner),
+				issuer: Some(self.issuer),
+				admin: Some(self.admin),
+				freezer: Some(self.freezer),
 				supply: self.supply,
 				deposit: self.deposit,
 				min_balance: self.min_balance,
@@ -120,3 +120,100 @@ pub mod v1 {
 		}
 	}
 }
+
+pub mod v2 {
+	use super::*;
+	use frame_support::{pallet_prelude::*, weights::Weight};
+
+	#[derive(Decode)]
+	pub struct OldAssetDetails<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> OldAssetDetails<Balance, AccountId, DepositBalance> {
+		fn migrate_to_v2(self) -> AssetDetails<Balance, AccountId, DepositBalance> {
+			AssetDetails {
+				owner: Some(self.owner),
+				issuer: Some(self.issuer),
+				admin: Some(self.admin),
+				freezer: Some(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: self.status,
+			}
+		}
+	}
+
+	pub struct MigrateToV2<T>(sp_std::marker::PhantomData<T>);
+	impl<T: Config> OnRuntimeUpgrade for MigrateToV2<T> {
+		fn on_runtime_upgrade() -> Weight {
+			let current_version = Pallet::<T>::current_storage_version();
+			let onchain_version = Pallet::<T>::on_chain_storage_version();
+			if onchain_version == 0 && current_version == 2 {
+				let mut translated = 0u64;
+				Asset::<T>::translate::<
+					OldAssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T>>,
+					_,
+				>(|_key, old_value| {
+					translated.saturating_inc();
+					Some(old_value.migrate_to_v2())
+				});
+				current_version.put::<Pallet<T>>();
+				log::info!(target: "runtime::assets", "Upgraded {} pools, storage to version {:?}", translated, current_version);
+				T::DbWeight::get().reads_writes(translated + 1, translated + 1)
+			} else {
+				log::info!(target: "runtime::assets",  "Migration did not execute. This probably should be removed");
+				T::DbWeight::get().reads(1)
+			}
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+			frame_support::ensure!(
+				Pallet::<T>::on_chain_storage_version() == 0,
+				"must upgrade linearly"
+			);
+			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<(), &'static str> {
+			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;
+			assert_eq!(
+				prev_count, post_count,
+				"the asset count before and after the migration should be the same"
+			);
+
+			let current_version = Pallet::<T>::current_storage_version();
+			let onchain_version = Pallet::<T>::on_chain_storage_version();
+
+			frame_support::ensure!(current_version == 1, "must_upgrade");
+			assert_eq!(
+				current_version, onchain_version,
+				"after migration, the current_version and onchain_version should be the same"
+			);
+
+			Ok(())
+		}
+	}
+}