-
Notifications
You must be signed in to change notification settings - Fork 382
Girazoki avoid statemine prefix breaking change #1159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e170f4c
ef6ae21
32cdb42
a15613b
9d44c8f
bf39e2e
aeef2b3
306bb0e
6bc863c
01b0618
bdeaebf
14c2599
8b4dcbb
8aa9b70
c7112a7
1895e6c
723dd97
1b4e8e0
5ec2c1f
2667fef
982b6ec
fd2e0dc
34344aa
82f30eb
67b0aae
13852c4
5c82ee9
c0fc100
d2694d0
e0cb77f
e31d450
cdc8f42
42399ad
fff3b82
94dbdfa
eccdb27
555d0bf
b23dcf7
437d089
f7ef5c0
a8e899c
ee85d85
c025e17
c7cdf07
988eb26
c7650bd
90a2026
8392853
9a97266
239964b
4b8faef
08452c5
5086d17
fc91d87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,20 +21,24 @@ | |
| //! The main goal of this pallet is to allow moonbeam to register XCM assets | ||
| //! The assumption is we work with AssetTypes, which can then be comperted to AssetIds | ||
| //! | ||
| //! This pallet has two storage items: AssetIdType, which holds a mapping from AssetId->AssetType | ||
| //! AssetIdUnitsPerSecond: an AssetId->u128 mapping that holds how much each AssetId should be | ||
| //! charged per unit of second, in the case such an Asset is received as a XCM asset. | ||
| //! This pallet has three storage items: AssetIdType, which holds a mapping from AssetId->AssetType | ||
| //! AssetTypeUnitsPerSecond: an AssetType->u128 mapping that holds how much each AssetType should be | ||
| //! charged per unit of second, in the case such an Asset is received as a XCM asset. Finally, | ||
| //! AssetTypeId holds a mapping from AssetType -> AssetId. | ||
| //! | ||
| //! This pallet has two extrinsics: register_asset, which registers an Asset in this pallet and | ||
| //! This pallet has three extrinsics: register_asset, which registers an Asset in this pallet and | ||
| //! creates the asset as dictated by the AssetRegistrar trait. set_asset_units_per_second: which | ||
| //! sets the unit per second that should be charged for a particular asset. | ||
| //! change_existing_asset_type: which allows to update the correspondence between AssetId and | ||
| //! AssetType | ||
|
|
||
| #![cfg_attr(not(feature = "std"), no_std)] | ||
|
|
||
| use frame_support::pallet; | ||
| pub use pallet::*; | ||
| #[cfg(any(test, feature = "runtime-benchmarks"))] | ||
| mod benchmarks; | ||
| pub mod migrations; | ||
| #[cfg(test)] | ||
| pub mod mock; | ||
| #[cfg(test)] | ||
|
|
@@ -73,11 +77,15 @@ pub mod pallet { | |
| fn get_asset_type(asset_id: T::AssetId) -> Option<T::AssetType> { | ||
| AssetIdType::<T>::get(asset_id) | ||
| } | ||
|
|
||
| fn get_asset_id(asset_type: T::AssetType) -> Option<T::AssetId> { | ||
| AssetTypeId::<T>::get(asset_type) | ||
| } | ||
|
Comment on lines
+81
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This implies a 1:1 mapping between an
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is wrong. We need the asset-manager to provide one specific multiLocation when a user says: "I want to transact asset X".
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assetType exists mainly to be able to reference distinct type of tokens in the future. Right now its an enum containing XCM(MultiLocation) but in the future we might want to distinguish between, e.g., fungible and non-fungible tokens, which should probably go into different pallets. But since our approach is to retrieve the Id through a hash, there is low risk of collision
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, that's right. This constraint is about communicating back to another chain, right?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes |
||
| } | ||
|
|
||
| impl<T: Config> xcm_primitives::UnitsToWeightRatio<T::AssetId> for Pallet<T> { | ||
| fn get_units_per_second(asset_id: T::AssetId) -> Option<u128> { | ||
| AssetIdUnitsPerSecond::<T>::get(asset_id) | ||
| impl<T: Config> xcm_primitives::UnitsToWeightRatio<T::AssetType> for Pallet<T> { | ||
| fn get_units_per_second(asset_type: T::AssetType) -> Option<u128> { | ||
| AssetTypeUnitsPerSecond::<T>::get(asset_type) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -118,19 +126,32 @@ pub mod pallet { | |
| #[pallet::generate_deposit(pub(crate) fn deposit_event)] | ||
| pub enum Event<T: Config> { | ||
| AssetRegistered(T::AssetId, T::AssetType, T::AssetRegistrarMetadata), | ||
| UnitsPerSecondChanged(T::AssetId, u128), | ||
| UnitsPerSecondChanged(T::AssetType, u128), | ||
| AssetTypeChanged(T::AssetId, T::AssetType), | ||
| } | ||
|
|
||
| /// Stores the asset TYPE | ||
| /// Mapping from an asset id to asset type. | ||
| /// This is mostly used when receiving transaction specifying an asset directly, | ||
| /// like transferring an asset from this chain to another. | ||
| #[pallet::storage] | ||
| #[pallet::getter(fn asset_id_type)] | ||
| pub type AssetIdType<T: Config> = StorageMap<_, Blake2_128Concat, T::AssetId, T::AssetType>; | ||
|
|
||
| // Stores the units per second for local execution. | ||
| // Not all assets might contain units per second, hence the different storage | ||
| /// Reverse mapping of AssetIdType. Mapping from an asset type to an asset id. | ||
| /// This is mostly used when receiving a multilocation XCM message to retrieve | ||
| /// the corresponding asset in which tokens should me minted. | ||
| #[pallet::storage] | ||
| #[pallet::getter(fn asset_id_units_per_second)] | ||
| pub type AssetIdUnitsPerSecond<T: Config> = StorageMap<_, Blake2_128Concat, T::AssetId, u128>; | ||
| #[pallet::getter(fn asset_type_id)] | ||
| pub type AssetTypeId<T: Config> = StorageMap<_, Blake2_128Concat, T::AssetType, T::AssetId>; | ||
|
|
||
| /// Stores the units per second for local execution for a AssetType. | ||
| /// This is used to know how to charge for XCM execution in a particular | ||
| /// asset | ||
| /// Not all assets might contain units per second, hence the different storage | ||
| #[pallet::storage] | ||
| #[pallet::getter(fn asset_type_units_per_second)] | ||
| pub type AssetTypeUnitsPerSecond<T: Config> = | ||
| StorageMap<_, Blake2_128Concat, T::AssetType, u128>; | ||
|
|
||
| #[pallet::call] | ||
| impl<T: Config> Pallet<T> { | ||
|
|
@@ -154,6 +175,7 @@ pub mod pallet { | |
| .map_err(|_| Error::<T>::ErrorCreatingAsset)?; | ||
|
|
||
| AssetIdType::<T>::insert(&asset_id, &asset); | ||
| AssetTypeId::<T>::insert(&asset, &asset_id); | ||
|
|
||
| Self::deposit_event(Event::AssetRegistered(asset_id, asset, metadata)); | ||
| Ok(()) | ||
|
|
@@ -163,19 +185,42 @@ pub mod pallet { | |
| #[pallet::weight(T::WeightInfo::set_asset_units_per_second())] | ||
| pub fn set_asset_units_per_second( | ||
| origin: OriginFor<T>, | ||
| asset_id: T::AssetId, | ||
| asset_type: T::AssetType, | ||
| units_per_second: u128, | ||
| ) -> DispatchResult { | ||
| T::AssetModifierOrigin::ensure_origin(origin)?; | ||
|
|
||
| ensure!( | ||
| AssetIdType::<T>::get(&asset_id).is_some(), | ||
| AssetTypeId::<T>::get(&asset_type).is_some(), | ||
| Error::<T>::AssetDoesNotExist | ||
| ); | ||
|
|
||
| AssetIdUnitsPerSecond::<T>::insert(&asset_id, &units_per_second); | ||
| AssetTypeUnitsPerSecond::<T>::insert(&asset_type, &units_per_second); | ||
|
|
||
| Self::deposit_event(Event::UnitsPerSecondChanged(asset_type, units_per_second)); | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Change the xcm type mapping for a given assetId | ||
| #[pallet::weight(T::WeightInfo::set_asset_units_per_second())] | ||
| pub fn change_existing_asset_type( | ||
| origin: OriginFor<T>, | ||
| asset_id: T::AssetId, | ||
| new_asset_type: T::AssetType, | ||
| ) -> DispatchResult { | ||
| T::AssetModifierOrigin::ensure_origin(origin)?; | ||
|
|
||
| let previous_asset_type = | ||
| AssetIdType::<T>::get(&asset_id).ok_or(Error::<T>::AssetDoesNotExist)?; | ||
|
|
||
| // Insert new asset type info | ||
| AssetIdType::<T>::insert(&asset_id, &new_asset_type); | ||
| AssetTypeId::<T>::insert(&new_asset_type, &asset_id); | ||
|
|
||
| // Remove previous asset type info | ||
| AssetTypeId::<T>::remove(&previous_asset_type); | ||
|
|
||
| Self::deposit_event(Event::UnitsPerSecondChanged(asset_id, units_per_second)); | ||
| Self::deposit_event(Event::AssetTypeChanged(asset_id, new_asset_type)); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's (1) changing from one asset type to the same asset type, and (2) expecting the implementation of
change_existing_asset_type()to not short-circuit this case.This works for now because that is actually how
change_existing_asset_type()is implemented, but it's probably more future-proof to write this test so that it actually changes the type (that way a future "optimization" or similar won't cause the benchmark to become ineffective).Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to write this benchmark like you described, but I have two associated types (assetId, assetType) that I need to generate and in order to be generic (over the mock runtime and over the proper runtime where this is included) I can only generate the default one. Still I think the benchmarking is correct
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One option is to require that
AssetType: From<MultiLocation>in the config, and in that case I can generate several. But to be honest the pallet is so simple that I did not even bother requiring this. Let me know if this is something we think we should addThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I don't think my concern is worth a lot of effort...