Skip to content
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.

Sign up for GitHub

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

wip: Don't let user provide min_balance or is_sufficient as parameters to update_asset_metadata #1062

Open
wants to merge 4 commits into
base: manta
Choose a base branch
from
Open

wip: Don't let user provide min_balance or is_sufficient as parameters to update_asset_metadata #1062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a285578
wip: Change to take as parameter
Apr 4, 2023
5f10571
finish implementation, no need to insert into AssetIdMetadata
Apr 11, 2023
d894319
Merge branch 'manta' into garandor/asset_metadata2
Apr 11, 2023
cbadece
fix benchmark
Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pallets/asset-manager/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ benchmarks! {
let metadata = <T::AssetConfig as AssetConfig<T>>::AssetRegistryMetadata::testing_default();
Pallet::<T>::register_asset(RawOrigin::Root.into(), location, metadata.clone())?;
let some_valid_asset_id = <T as Config>::AssetId::from(assets_count);
}: _(RawOrigin::Root, some_valid_asset_id, metadata.clone())
}: _(RawOrigin::Root, some_valid_asset_id, metadata.clone().into())
verify {
assert_last_event::<T>(Event::AssetMetadataUpdated { asset_id: some_valid_asset_id, metadata }.into());
}
Expand Down
13 changes: 8 additions & 5 deletions pallets/asset-manager/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ pub mod pallet {
pub fn update_asset_metadata(
origin: OriginFor<T>,
asset_id: T::AssetId,
metadata: <T::AssetConfig as AssetConfig<T>>::AssetRegistryMetadata,
metadata: <<T as Config>::AssetConfig as AssetConfig<T>>::StorageMetadata,
) -> DispatchResult {
T::ModifierOrigin::ensure_origin(origin)?;
ensure!(
Expand All @@ -461,11 +461,14 @@ pub mod pallet {
Error::<T>::UpdateNonExistentAsset
);
<T::AssetConfig as AssetConfig<T>>::AssetRegistry::update_asset_metadata(
&asset_id,
metadata.clone().into(),
&asset_id, metadata,
)?;
AssetIdMetadata::<T>::insert(asset_id, &metadata);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests are passing but I'm 90% sure it's not safe to skip this insert. The Assets metadata got updated above and it never seems to be used from here (not sure why we store the AssetStorageMetadata anyway since we never do anything with it outside of Assets code) but frontend code might depend on asset_id_metadata returning the correct values somewhere

The interface has no mutators unfortunately, so not sure how to update the field otherwise

Self::deposit_event(Event::<T>::AssetMetadataUpdated { asset_id, metadata });
let updated_registry_metadata = Self::asset_id_metadata(asset_id)
.expect("we just successfully updated the asset, so it exists. qed");
Self::deposit_event(Event::<T>::AssetMetadataUpdated {
asset_id,
metadata: updated_registry_metadata,
});
Ok(())
}

Expand Down
8 changes: 4 additions & 4 deletions pallets/asset-manager/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn wrong_modifier_origin_should_not_work() {
AssetManager::update_asset_metadata(
Origin::signed([3u8; 32].into()),
0,
asset_metadata
asset_metadata.into()
),
BadOrigin
);
Expand Down Expand Up @@ -166,14 +166,14 @@ fn update_asset() {
AssetManager::update_asset_metadata(
Origin::root(),
native_asset_id,
new_metadata.clone(),
new_metadata.clone().into(),
),
Error::<Runtime>::CannotUpdateNativeAssetMetadata
);
assert_ok!(AssetManager::update_asset_metadata(
Origin::root(),
asset_id,
new_metadata.clone(),
new_metadata.clone().into(),
),);
assert_eq!(Assets::name(&asset_id), new_name);
assert_eq!(Assets::symbol(&asset_id), new_symbol);
Expand Down Expand Up @@ -205,7 +205,7 @@ fn update_asset() {
AssetManager::update_asset_metadata(
Origin::root(),
next_asset_id,
new_metadata.clone()
new_metadata.clone().into()
),
Error::<Runtime>::UpdateNonExistentAsset
);
Expand Down
4 changes: 2 additions & 2 deletions primitives/manta/src/assets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub trait AssetRegistry: AssetIdType + BalanceType {

/// Update asset metadata by `AssetId`.
///
/// * `asset_id`: the asset id to be created.
/// * `asset_id`: the asset id to be updated.
/// * `metadata`: the metadata that the implementation layer stores.
fn update_asset_metadata(
asset_id: &Self::AssetId,
Expand All @@ -120,7 +120,7 @@ where
C: Config,
{
/// Metadata type that required in token storage: e.g. AssetMetadata in Pallet-Assets.
type StorageMetadata: From<Self::AssetRegistryMetadata>;
type StorageMetadata: From<Self::AssetRegistryMetadata> + Parameter;

/// The Asset Metadata type stored in this pallet.
type AssetRegistryMetadata: AssetMetadata<Balance = Self::Balance> + Parameter + TestingDefault;
Expand Down