Fix AssetManager's update_asset_metadata

pallets/asset-manager/src/lib.rs

@@ -45,7 +45,10 @@ pub mod pallet {
 		},
 		types::AssetId,
 	};
-	use sp_runtime::{traits::AccountIdConversion, ArithmeticError};
+	use sp_runtime::{
+		traits::{AccountIdConversion, One},
+		ArithmeticError,
+	};
 
 	#[pallet::pallet]
 	#[pallet::generate_store(pub(super) trait Store)]
@@ -125,6 +128,8 @@ pub mod pallet {
 		ErrorCreatingAsset,
 		/// Update a non-exist asset
 		UpdateNonExistAsset,
+		/// Cannot update reserved assets metadata (0 and 1)
+		CannotUpdateNativeAssetMetadata,
 		/// Asset already registered.
 		AssetAlreadyRegistered,
 	}
@@ -166,7 +171,7 @@ pub mod pallet {
 	impl<T: Config> Pallet<T> {
 		/// Register a new asset in the asset manager.
 		///
-		/// * `origin`: Caller of this extrinsic, the acess control is specfied by `ForceOrigin`.
+		/// * `origin`: Caller of this extrinsic, the access control is specified by `ForceOrigin`.
 		/// * `location`: Location of the asset.
 		/// * `metadata`: Asset metadata.
 		/// * `min_balance`: Minimum balance to keep an account alive, used in conjunction with `is_sufficient`.
@@ -209,7 +214,7 @@ pub mod pallet {
 
 		/// Update an asset by its asset id in the asset manager.
 		///
-		/// * `origin`: Caller of this extrinsic, the acess control is specfied by `ForceOrigin`.
+		/// * `origin`: Caller of this extrinsic, the access control is specified by `ForceOrigin`.
 		/// * `asset_id`: AssetId to be updated.
 		/// * `location`: `location` to update the asset location.
 		/// # <weight>
@@ -245,7 +250,7 @@ pub mod pallet {
 
 		/// Update an asset's metadata by its `asset_id`
 		///
-		/// * `origin`: Caller of this extrinsic, the acess control is specfied by `ForceOrigin`.
+		/// * `origin`: Caller of this extrinsic, the access control is specified by `ForceOrigin`.
 		/// * `asset_id`: AssetId to be updated.
 		/// * `metadata`: new `metadata` to be associated with `asset_id`.
 		#[pallet::weight(50_000_000)]
@@ -256,18 +261,26 @@ pub mod pallet {
 			metadata: <T::AssetConfig as AssetConfig>::AssetRegistrarMetadata,
 		) -> DispatchResult {
 			T::ModifierOrigin::ensure_origin(origin)?;
+			ensure!(
+				!asset_id.is_one(),
+				Error::<T>::CannotUpdateNativeAssetMetadata
+			);
 			ensure!(
 				AssetIdLocation::<T>::contains_key(&asset_id),
 				Error::<T>::UpdateNonExistAsset
 			);
+			<T::AssetConfig as AssetConfig>::AssetRegistrar::update_asset_metadata(
+				asset_id,
+				metadata.clone().into(),
+			)?;
 			AssetIdMetadata::<T>::insert(&asset_id, &metadata);
 			Self::deposit_event(Event::<T>::AssetMetadataUpdated { asset_id, metadata });
 			Ok(())
 		}
 
 		/// Update an asset by its asset id in the asset manager.
 		///
-		/// * `origin`: Caller of this extrinsic, the acess control is specfied by `ForceOrigin`.
+		/// * `origin`: Caller of this extrinsic, the access control is specified by `ForceOrigin`.
 		/// * `asset_id`: AssetId to be updated.
 		/// * `units_per_second`: units per second for `asset_id`
 		/// # <weight>

pallets/asset-manager/src/tests.rs

@@ -13,16 +13,14 @@
 //
 // You should have received a copy of the GNU General Public License
 // along with Manta.  If not, see <http://www.gnu.org/licenses/>.
-//
-// The pallet-tx-pause pallet is forked from Acala's transaction-pause module https://github.com/AcalaNetwork/Acala/tree/master/modules/transaction-pause
-// The original license is the following - SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
 
 //! unit tests for asset-manager
 
 use crate::{self as asset_manager, AssetIdLocation, UnitsPerSecond};
 use asset_manager::mock::*;
-use frame_support::{assert_noop, assert_ok};
+use frame_support::{assert_noop, assert_ok, traits::fungibles::InspectMetadata};
 use manta_primitives::assets::{AssetLocation, AssetRegistrarMetadata};
+
 use sp_runtime::traits::BadOrigin;
 use xcm::{latest::prelude::*, VersionedMultiLocation};
 
@@ -33,7 +31,7 @@ fn basic_setup_should_work() {
 }
 
 #[test]
-fn wrong_modifer_origin_should_not_work() {
+fn wrong_modifier_origin_should_not_work() {
 	new_test_ext().execute_with(|| {
 		let asset_metadata = AssetRegistrarMetadata {
 			name: b"Kusama".to_vec(),
@@ -122,69 +120,150 @@ fn register_asset_should_work() {
 
 #[test]
 fn update_asset() {
-	let asset_metadata = AssetRegistrarMetadata {
-		name: b"Kusama".to_vec(),
-		symbol: b"KSM".to_vec(),
+	let dummy_asset_id = 0;
+	let dummy_asset_metadata = AssetRegistrarMetadata {
+		name: b"Dummy".to_vec(),
+		symbol: b"DMY".to_vec(),
 		decimals: 12,
 		min_balance: 1u128,
 		evm_address: None,
 		is_frozen: false,
 		is_sufficient: true,
 	};
+	let dummy_location = AssetLocation(VersionedMultiLocation::V1(MultiLocation::new(
+		1,
+		X1(Parachain(0)),
+	)));
+
+	let native_asset_id = 1;
+	let native_asset_metadata = AssetRegistrarMetadata {
+		name: b"Calamari".to_vec(),
+		symbol: b"KMA".to_vec(),
+		decimals: 12,
+		min_balance: 1u128,
+		evm_address: None,
+		is_frozen: false,
+		is_sufficient: true,
+	};
+	let self_location = AssetLocation(VersionedMultiLocation::V1(MultiLocation::new(
+		1,
+		X1(Parachain(1)),
+	)));
+
+	let ksm_asset_id = 2;
+	let original_name = b"Kusama".to_vec();
+	let original_symbol = b"KSM".to_vec();
+	let original_decimals = 12;
+	let asset_metadata = AssetRegistrarMetadata {
+		name: original_name,
+		symbol: original_symbol,
+		decimals: original_decimals,
+		min_balance: 1u128,
+		evm_address: None,
+		is_frozen: false,
+		is_sufficient: true,
+	};
 	let mut new_metadata = asset_metadata.clone();
-	new_metadata.is_frozen = true;
+	let new_name = b"NotKusama".to_vec();
+	let new_symbol = b"NotKSM".to_vec();
+	let new_decimals = original_decimals + 1;
+	new_metadata.name = new_name.clone();
+	new_metadata.symbol = new_symbol.clone();
+	new_metadata.decimals = new_decimals;
 	let source_location = AssetLocation(VersionedMultiLocation::V1(MultiLocation::parent()));
 	let new_location = AssetLocation(VersionedMultiLocation::V1(MultiLocation::new(
 		1,
 		X2(Parachain(1), PalletInstance(PALLET_BALANCES_INDEX)),
 	)));
 	new_test_ext().execute_with(|| {
+		// Register the dummy asset
+		assert_ok!(AssetManager::register_asset(
+			Origin::root(),
+			dummy_location.clone(),
+			dummy_asset_metadata.clone()
+		));
+		assert_eq!(
+			AssetIdLocation::<Runtime>::get(dummy_asset_id),
+			Some(dummy_location.clone())
+		);
+		// Register the native token
+		assert_ok!(AssetManager::register_asset(
+			Origin::root(),
+			self_location.clone(),
+			native_asset_metadata.clone()
+		));
+		assert_eq!(
+			AssetIdLocation::<Runtime>::get(native_asset_id),
+			Some(self_location.clone())
+		);
 		// Register relay chain native token
 		assert_ok!(AssetManager::register_asset(
 			Origin::root(),
 			source_location.clone(),
 			asset_metadata.clone()
 		));
 		assert_eq!(
-			AssetIdLocation::<Runtime>::get(0),
+			AssetIdLocation::<Runtime>::get(ksm_asset_id),
 			Some(source_location.clone())
 		);
-		// Update the asset metadata
+		// Cannot update asset 1. Will be reserved for the native asset.
+		assert_noop!(
+			AssetManager::update_asset_metadata(
+				Origin::root(),
+				native_asset_id,
+				new_metadata.clone(),
+			),
+			crate::Error::<Runtime>::CannotUpdateNativeAssetMetadata
+		);
 		assert_ok!(AssetManager::update_asset_metadata(
 			Origin::root(),
-			0,
-			new_metadata.clone()
-		));
+			ksm_asset_id,
+			new_metadata.clone(),
+		),);
+		assert_eq!(Assets::name(&ksm_asset_id), new_name);
+		assert_eq!(Assets::symbol(&ksm_asset_id), new_symbol);
+		assert_eq!(Assets::decimals(&ksm_asset_id), new_decimals);
 		// Update the asset location
 		assert_ok!(AssetManager::update_asset_location(
 			Origin::root(),
-			0,
+			ksm_asset_id,
 			new_location.clone()
 		));
 		// Update asset units per seconds
 		assert_ok!(AssetManager::set_units_per_second(
 			Origin::root(),
-			0,
+			ksm_asset_id,
 			125u128
 		));
-		assert_eq!(UnitsPerSecond::<Runtime>::get(0), Some(125));
+		assert_eq!(UnitsPerSecond::<Runtime>::get(ksm_asset_id), Some(125));
+		let next_asset_id = 3;
 		// Update a non-exist asset should fail
 		assert_noop!(
-			AssetManager::update_asset_location(Origin::root(), 1, new_location.clone()),
+			AssetManager::update_asset_location(
+				Origin::root(),
+				next_asset_id,
+				new_location.clone()
+			),
 			crate::Error::<Runtime>::UpdateNonExistAsset
 		);
 		assert_noop!(
-			AssetManager::update_asset_metadata(Origin::root(), 1, new_metadata.clone()),
+			AssetManager::update_asset_metadata(
+				Origin::root(),
+				next_asset_id,
+				new_metadata.clone()
+			),
 			crate::Error::<Runtime>::UpdateNonExistAsset
 		);
-		// Update an asset to an existing location will fail
+		// Re-registering the original location and metadata should work,
+		// as we modified the previous asset.
 		assert_ok!(AssetManager::register_asset(
 			Origin::root(),
 			source_location.clone(),
 			asset_metadata.clone()
 		));
+		// But updating the asset to an existing location will fail.
 		assert_noop!(
-			AssetManager::update_asset_location(Origin::root(), 1, new_location),
+			AssetManager::update_asset_location(Origin::root(), next_asset_id, new_location),
 			crate::Error::<Runtime>::LocationAlreadyExists
 		);
 	})