asset-registry: Pass Asset Id to AuthorityOrigin (#762)

* asset-registry: Pass Asset Id to AuthorityOrigin

Different AssetId's may require different origin checks, which is
currently not possible. These changes use EnsureOriginWithArg to allow
users of this pallet to tailor the origin check to take the `asset_id`
itself in consideration when choosing the specific `EnsureOrigin`
implementation to be applied.

* fixup!

* Update asset-registry/src/mock/para.rs

* Update asset-registry/src/mock/para.rs

* fmt

Co-authored-by: Shaun Wang <[email protected]>
Co-authored-by: Bryan Chen <[email protected]>

Author: 3 people

asset-registry/src/lib.rs

@@ -2,8 +2,7 @@
 // Older clippy versions give a false positive on the expansion of [pallet::call].
 // This is fixed in https://github.com/rust-lang/rust-clippy/issues/8321
 #![allow(clippy::large_enum_variant)]
-
-use frame_support::{pallet_prelude::*, traits::EnsureOrigin, transactional};
+use frame_support::{pallet_prelude::*, traits::EnsureOriginWithArg, transactional};
 use frame_system::pallet_prelude::*;
 use orml_traits::asset_registry::AssetProcessor;
 use scale_info::TypeInfo;
@@ -19,9 +18,12 @@ pub use module::*;
 pub use weights::WeightInfo;
 
 mod impls;
+mod weights;
+
+#[cfg(test)]
 mod mock;
+#[cfg(test)]
 mod tests;
-mod weights;
 
 /// Data describing the asset properties.
 #[derive(scale_info::TypeInfo, Encode, Decode, Clone, Eq, PartialEq, RuntimeDebug)]
@@ -48,8 +50,8 @@ pub mod module {
 		/// The type used as a unique asset id,
 		type AssetId: Parameter + Member + Default + TypeInfo;
 
-		/// The origin that is allowed to manipulate metadata.
-		type AuthorityOrigin: EnsureOrigin<<Self as frame_system::Config>::Origin>;
+		/// Checks that an origin has the authority to register/update an asset
+		type AuthorityOrigin: EnsureOriginWithArg<Self::Origin, Option<Self::AssetId>>;
 
 		/// A filter ran upon metadata registration that assigns an is and
 		/// potentially modifies the supplied metadata.
@@ -88,10 +90,6 @@ pub mod module {
 			asset_id: T::AssetId,
 			metadata: AssetMetadata<T::Balance, T::CustomMetadata>,
 		},
-		SetLocation {
-			asset_id: T::AssetId,
-			location: Box<VersionedMultiLocation>,
-		},
 	}
 
 	/// The metadata of an asset, indexed by asset id.
@@ -147,7 +145,7 @@ pub mod module {
 			metadata: AssetMetadata<T::Balance, T::CustomMetadata>,
 			asset_id: Option<T::AssetId>,
 		) -> DispatchResult {
-			T::AuthorityOrigin::ensure_origin(origin)?;
+			T::AuthorityOrigin::ensure_origin(origin, &asset_id)?;
 
 			Self::do_register_asset(metadata, asset_id)
 		}
@@ -165,7 +163,7 @@ pub mod module {
 			location: Option<Option<VersionedMultiLocation>>,
 			additional: Option<T::CustomMetadata>,
 		) -> DispatchResult {
-			T::AuthorityOrigin::ensure_origin(origin)?;
+			T::AuthorityOrigin::ensure_origin(origin, &Some(asset_id.clone()))?;
 
 			Self::do_update_asset(
 				asset_id,

asset-registry/src/mock/para.rs

@@ -4,13 +4,14 @@ use crate as orml_asset_registry;
 
 use codec::{Decode, Encode};
 use cumulus_primitives_core::{ChannelStatus, GetChannelInfo, ParaId};
+use frame_support::traits::{EnsureOrigin, EnsureOriginWithArg};
 use frame_support::{
-	construct_runtime, match_types, parameter_types,
+	construct_runtime, match_types, ord_parameter_types, parameter_types,
 	traits::{ConstU128, ConstU32, ConstU64, Everything, Nothing},
 	weights::{constants::WEIGHT_PER_SECOND, Weight},
 	PalletId,
 };
-use frame_system::EnsureRoot;
+use frame_system::{EnsureRoot, EnsureSignedBy};
 use orml_asset_registry::{AssetRegistryTrader, FixedRateAssetRegistryTrader};
 use orml_traits::{
 	location::{AbsoluteReserveProvider, RelativeReserveProvider},
@@ -105,11 +106,39 @@ pub struct CustomMetadata {
 	pub fee_per_second: u128,
 }
 
+const ADMIN_ASSET_TWO: AccountId = AccountId32::new([42u8; 32]);
+
+ord_parameter_types! {
+	pub const AdminAssetTwo: AccountId = ADMIN_ASSET_TWO;
+}
+
+pub struct AssetAuthority;
+impl EnsureOriginWithArg<Origin, Option<u32>> for AssetAuthority {
+	type Success = ();
+
+	fn try_origin(origin: Origin, asset_id: &Option<u32>) -> Result<Self::Success, Origin> {
+		match asset_id {
+			// We mock an edge case where the asset_id 2 requires a special origin check.
+			Some(2) => EnsureSignedBy::<AdminAssetTwo, AccountId32>::try_origin(origin.clone())
+				.map(|_| ())
+				.map_err(|_| origin),
+
+			// Any other `asset_id` defaults to EnsureRoot
+			_ => EnsureRoot::try_origin(origin),
+		}
+	}
+
+	#[cfg(feature = "runtime-benchmarks")]
+	fn successful_origin(_asset_id: &Option<u32>) -> Origin {
+		unimplemented!()
+	}
+}
+
 impl orml_asset_registry::Config for Runtime {
 	type Event = Event;
 	type Balance = Balance;
 	type AssetId = u32;
-	type AuthorityOrigin = EnsureRoot<AccountId>;
+	type AuthorityOrigin = AssetAuthority;
 	type CustomMetadata = CustomMetadata;
 	type AssetProcessor = orml_asset_registry::SequentialId<Runtime>;
 	type WeightInfo = ();

asset-registry/src/tests.rs

@@ -2,12 +2,16 @@
 
 use super::*;
 use crate as orml_asset_registry;
-use crate::tests::para::{AssetRegistry, CustomMetadata, Origin, Tokens, TreasuryAccount};
+use crate::tests::para::{AdminAssetTwo, AssetRegistry, CustomMetadata, Origin, Tokens, TreasuryAccount};
 use frame_support::{assert_noop, assert_ok};
 use mock::*;
 use orml_traits::MultiCurrency;
 use polkadot_parachain::primitives::Sibling;
-use sp_runtime::{traits::AccountIdConversion, AccountId32};
+
+use sp_runtime::{
+	traits::{AccountIdConversion, BadOrigin},
+	AccountId32,
+};
 use xcm_simulator::TestExt;
 
 fn treasury_account() -> AccountId32 {
@@ -458,3 +462,33 @@ fn test_existential_deposits() {
 		);
 	});
 }
+
+#[test]
+fn test_asset_authority() {
+	TestNet::reset();
+
+	ParaA::execute_with(|| {
+		let metadata = dummy_metadata();
+
+		// Assert that root can register an asset with id 1
+		assert_ok!(AssetRegistry::register_asset(Origin::root(), metadata.clone(), Some(1)));
+
+		// Assert that only Account42 can register asset with id 42
+		let metadata = AssetMetadata {
+			location: None,
+			..dummy_metadata()
+		};
+
+		// It fails when signed with root...
+		assert_noop!(
+			AssetRegistry::register_asset(Origin::root(), metadata.clone(), Some(2)),
+			BadOrigin
+		);
+		// It works when signed with the right account
+		assert_ok!(AssetRegistry::register_asset(
+			Origin::signed(AdminAssetTwo::get()),
+			metadata,
+			Some(2)
+		));
+	});
+}