Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Metadata in pallet-assets cannot be accessed by other pallet #9877

Closed
mclyk opened this issue Sep 28, 2021 · 10 comments · Fixed by #9893
Closed

Metadata in pallet-assets cannot be accessed by other pallet #9877

mclyk opened this issue Sep 28, 2021 · 10 comments · Fixed by #9893
Labels
J2-unconfirmed Issue might be valid, but it’s not yet known.

Comments

@mclyk
Copy link

mclyk commented Sep 28, 2021

Hi, I have a question about pallet-assets.
we are developing a DeFi project that need to use asset's decimal when calculating, however when we import pallet-assets in our project, the storage Metadata can not be accessed by other pallets.

#[pallet::storage]
/// Metadata of an asset.
pub(super) type Metadata<T: Config<I>, I: 'static = ()> = StorageMap<
        _,
        Blake2_128Concat,
        T::AssetId,
        AssetMetadata<DepositBalanceOf<T, I>, BoundedVec<u8, T::StringLimit>>,
        ValueQuery,
        GetDefault,
        ConstU32<300_000>,
>;

I wonder can we make it public or abstract a trait, so the other pallet can access its decimal?

@github-actions github-actions bot added the J2-unconfirmed Issue might be valid, but it’s not yet known. label Sep 28, 2021
@bkchr
Copy link
Member

bkchr commented Sep 28, 2021

CC @shawntabrizi @apopiak

@gui1117
Copy link
Contributor

gui1117 commented Sep 28, 2021

do you only need a read access ?
adding a getter can be the best way with #[pallet::getter(fn metadata)]

@mclyk
Copy link
Author

mclyk commented Sep 29, 2021

Yes, read access is enough, it seems like decimal can not be read even add #[pallet::getter(fn metadata)], because of the type AssetMetadata.

#[derive(Clone, Encode, Decode, Eq, PartialEq, Default, RuntimeDebug, MaxEncodedLen)]
pub struct AssetMetadata<DepositBalance, BoundedString> {
	/// The balance deposited for this metadata.
	///
	/// This pays for the data stored in this struct.
	pub(super) deposit: DepositBalance,
	/// The user friendly name of this asset. Limited in length by `StringLimit`.
	pub(super) name: BoundedString,
	/// The ticker symbol for this asset. Limited in length by `StringLimit`.
	pub(super) symbol: BoundedString,
	/// The number of decimals this asset uses to represent one unit.
	pub(super) decimals: u8,
	/// Whether the asset metadata may be changed by a non Force origin.
	pub(super) is_frozen: bool,
}

@mclyk
Copy link
Author

mclyk commented Sep 29, 2021

Now we define the decimal with hardcode in runtime to solve the problem, but this is error-prone.
our project is going to be a parachain soon, please help us with this if possible, thanks.

@shawntabrizi
Copy link
Member

@MrPai in what way are you using the decimal information in the runtime?

In general this sounds really bad because the runtime should never be doing floating point logic. The decimal, and all of the metadata should be used exclusively for front-end experiences outside of the blockchain.

@mclyk
Copy link
Author

mclyk commented Sep 29, 2021

Hi, @shawntabrizi

We have not done any floating-point logic on-chain. we used FixedU128 instead. the reason why we need decimal is that in our money market pallet, we need to calculate the total amount by price * amount.

As you know, USDT may have 6 decimal while KSM has 12, and DOT has 10.
Well when on-chain, the amount looks like this:

  • one USDT equals 1_000_000,
  • one KSM equals 1_000_000_000_000,
  • one DOT equals 10_000_000_000,

when oracle feed price to on-chain, the price looks like this:

  • USDT/USD : 1.00064
  • DOT/USD : 27.357
  • KSM/USD : 331.41
    and all the price wrap in FixedU128, so on-chain, it looks like:
  • USDT/USD : FixedU128(1,000,640,000,000,000,000)
  • DOT/USD : FixedU128(27,357,815,270,000,009,216)
  • KSM/USD : FixedU128(331,419,885,270,000,009,216)

IMPORTANT
From the above data, we cannot directly use price * amount due to the decimal difference, our formula looks like (price/decimal) * amount.

This is the reason why we need the decimal on-chain.

Can you help solve this requirement, we just change from orml-currency to pallet-assets, please let us know if you have any advice, thank you.

@shawntabrizi
Copy link
Member

shawntabrizi commented Sep 29, 2021

I have opened a simple PR to help you here, but in general, hard-coding things might not be such a bad idea. For example, how in general can you determine which asset corresponds to KSM, DOT, and USDT?

At that level, you must program that Asset ID X maps to some specific asset you care about, and in that case, it probably makes sense to hardcode all of the metadata about that asset in the interface. But anyway, hopefully that PR will solve things the way you envision.

@mclyk
Copy link
Author

mclyk commented Sep 29, 2021

Yes, the PR helps a lot, we are thinking about dynamically specifying and associate assets in our own pallet. and it will be easy for us to maintain decimal only in one place, pallet_assets here. No need to care about decimal at two places(runtime and pallet-assets).
A big thanks for your help :)

@ghost ghost closed this as completed in #9893 Sep 30, 2021
@GopherJ
Copy link
Contributor

GopherJ commented Oct 12, 2021

@shawntabrizi just one question here, I just checked that your pr is not in polkadot-v0.9.11, will we have chance to see it enters polkadot-v0.9.12 OR polkadot-v1.0.0? we kind of need it

@apopiak
Copy link
Contributor

apopiak commented Oct 12, 2021

The PR will make it into the next release that branches off master, so 0.9.12 seems likely

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J2-unconfirmed Issue might be valid, but it’s not yet known.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants