From 9deeaef2523214b4b059a6342ea79d345e47584b Mon Sep 17 00:00:00 2001 From: nuno Date: Sun, 5 Jun 2022 15:00:58 +0200 Subject: [PATCH 1/5] 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. --- asset-registry/src/lib.rs | 20 +++++++++---------- asset-registry/src/mock/para.rs | 35 ++++++++++++++++++++++++++++++--- asset-registry/src/tests.rs | 33 ++++++++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/asset-registry/src/lib.rs b/asset-registry/src/lib.rs index 21961a25e..d07380f14 100644 --- a/asset-registry/src/lib.rs +++ b/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<::Origin>; + /// Checks that an origin has the authority to register/update an asset + type AuthorityOrigin: EnsureOriginWithArg>; /// 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, }, - SetLocation { - asset_id: T::AssetId, - location: Box, - }, } /// The metadata of an asset, indexed by asset id. @@ -147,7 +145,7 @@ pub mod module { metadata: AssetMetadata, asset_id: Option, ) -> DispatchResult { - T::AuthorityOrigin::ensure_origin(origin)?; + T::AuthorityOrigin::ensure_origin(origin, &asset_id.clone())?; Self::do_register_asset(metadata, asset_id) } @@ -165,7 +163,7 @@ pub mod module { location: Option>, additional: Option, ) -> DispatchResult { - T::AuthorityOrigin::ensure_origin(origin)?; + T::AuthorityOrigin::ensure_origin(origin, &Some(asset_id.clone()))?; Self::do_update_asset( asset_id, diff --git a/asset-registry/src/mock/para.rs b/asset-registry/src/mock/para.rs index 3af6b9180..9161d0e8d 100644 --- a/asset-registry/src/mock/para.rs +++ b/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}, @@ -103,11 +104,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> for AssetAuthority { + type Success = (); + + fn try_origin(origin: Origin, asset_id: &Option) -> Result { + match asset_id { + // We mock an edge case where the asset_id 2 requires a special origin check. + Some(2) => EnsureSignedBy::::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(a: &u32) -> OO { + unimplemented!() + } +} + impl orml_asset_registry::Config for Runtime { type Event = Event; type Balance = Balance; type AssetId = u32; - type AuthorityOrigin = EnsureRoot; + type AuthorityOrigin = AssetAuthority; type CustomMetadata = CustomMetadata; type AssetProcessor = orml_asset_registry::SequentialId; type WeightInfo = (); diff --git a/asset-registry/src/tests.rs b/asset-registry/src/tests.rs index 01a2b5622..86bd79db6 100644 --- a/asset-registry/src/tests.rs +++ b/asset-registry/src/tests.rs @@ -2,11 +2,12 @@ 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::BadOrigin; use sp_runtime::AccountId32; use xcm_simulator::TestExt; @@ -461,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) + )); + }); +} From 8857d538d574e4e84e0ff2df8e4222bfd81ccca3 Mon Sep 17 00:00:00 2001 From: nuno Date: Tue, 7 Jun 2022 08:35:52 +0000 Subject: [PATCH 2/5] fixup! --- asset-registry/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/asset-registry/src/lib.rs b/asset-registry/src/lib.rs index d07380f14..a3fc951d6 100644 --- a/asset-registry/src/lib.rs +++ b/asset-registry/src/lib.rs @@ -145,7 +145,7 @@ pub mod module { metadata: AssetMetadata, asset_id: Option, ) -> DispatchResult { - T::AuthorityOrigin::ensure_origin(origin, &asset_id.clone())?; + T::AuthorityOrigin::ensure_origin(origin, &asset_id)?; Self::do_register_asset(metadata, asset_id) } From ca00b06359c378cc20f1c56f85d60e5c0facb8f8 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Wed, 8 Jun 2022 11:35:11 +1200 Subject: [PATCH 3/5] Update asset-registry/src/mock/para.rs --- asset-registry/src/mock/para.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/asset-registry/src/mock/para.rs b/asset-registry/src/mock/para.rs index 9161d0e8d..403fd2d4b 100644 --- a/asset-registry/src/mock/para.rs +++ b/asset-registry/src/mock/para.rs @@ -127,7 +127,7 @@ impl EnsureOriginWithArg> for AssetAuthority { } #[cfg(feature = "runtime-benchmarks")] - fn successful_origin(a: &u32) -> OO { + fn successful_origin(a: &u32) -> Origin { unimplemented!() } } From eb4c3e4311716a035d894a4530416d57d3dcdc96 Mon Sep 17 00:00:00 2001 From: Nuno Alexandre Date: Wed, 8 Jun 2022 19:00:02 +0000 Subject: [PATCH 4/5] Update asset-registry/src/mock/para.rs --- asset-registry/src/mock/para.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/asset-registry/src/mock/para.rs b/asset-registry/src/mock/para.rs index 403fd2d4b..bd68387fc 100644 --- a/asset-registry/src/mock/para.rs +++ b/asset-registry/src/mock/para.rs @@ -127,7 +127,7 @@ impl EnsureOriginWithArg> for AssetAuthority { } #[cfg(feature = "runtime-benchmarks")] - fn successful_origin(a: &u32) -> Origin { + fn successful_origin(_asset_id: &Option) -> Origin { unimplemented!() } } From 409bea3461ce4b1d9e94976596fda24bf1c3886e Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 9 Jun 2022 10:55:29 +1200 Subject: [PATCH 5/5] fmt --- asset-registry/src/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/asset-registry/src/tests.rs b/asset-registry/src/tests.rs index 1091d67de..b92f0fab2 100644 --- a/asset-registry/src/tests.rs +++ b/asset-registry/src/tests.rs @@ -8,7 +8,10 @@ use mock::*; use orml_traits::MultiCurrency; use polkadot_parachain::primitives::Sibling; -use sp_runtime::{traits::{AccountIdConversion, BadOrigin}, AccountId32}; +use sp_runtime::{ + traits::{AccountIdConversion, BadOrigin}, + AccountId32, +}; use xcm_simulator::TestExt; fn treasury_account() -> AccountId32 {