Make spacewalk use Fungibles instead of MultiCurrency

[ashneverdawn]

• [ ] Replace occurrences of MultiCurrency with Fungibles
• [ ] If needed, update the runtime so orml_tokens and orml_currencies can still be used.

[ashneverdawn]

The point of this task is that we want any chain to be able to add spacewalk to their runtime, whether they are using pallet-assets or orml.

It turns out orml MultiCurrency which we are currently using actually already implements pallet-assets Fungibles, so that should make things easier for us.

In spacewalk, we want to get rid of all references to orml and use the pallet-assets equivalent instead. Almost all pallets have references to orml. These all need to be replaced. (The testchain runtime should still be able to use orml)

Fungibles is from pallet-assets. You can find it here: https://github.com/paritytech/substrate/tree/master/frame/assets

orml MultiCurrency implements Fungibles. See: https://github.com/open-web3-stack/open-runtime-module-library/blob/master/tokens/src/impls.rs

Add pallet-assets to cargo like this:

pallet-assets = { git = 'https://github.com/paritytech/substrate.git', branch = 'polkadot-v0.9.31', default-features = false }

[b-yap]

@ashneverdawn
Is this taking out both the orml-traits and orml-tokens? Did I understand it right: removing the orml_tokens::Config dependency of the pallet currency ?

[vadaynujra]

Hey team! Please add your planning poker estimate with Zenhub @ashneverdawn @b-yap @ebma

[b-yap]

@vadaynujra This is mostly replacing pallets, so a runtime upgrade is necessary.

[b-yap]

@ebma @ashneverdawn Okay now I think this ticket is a little bit tricky.

First off: I think I might need to overhaul the pallet-currency because it has a lot of orml dependencies;

it's not a simple replace as using pallet_asset's AssetId, because we want to use either pallet_asset or orml. Therefore I was thinking that this should be a type inside pallet-currency's Config. https://github.com/pendulum-chain/spacewalk/blob/e76f4d46ce59811c00f557e74593f22e3a304d66/pallets/currency/src/types.rs#L6-L8

this has to go https://github.com/pendulum-chain/spacewalk/blob/e76f4d46ce59811c00f557e74593f22e3a304d66/pallets/currency/src/lib.rs#L61-L62

however, it's being used implicitly: https://github.com/pendulum-chain/spacewalk/blob/e76f4d46ce59811c00f557e74593f22e3a304d66/pallets/currency/src/lib.rs#L180-L199

https://github.com/pendulum-chain/spacewalk/blob/e76f4d46ce59811c00f557e74593f22e3a304d66/pallets/currency/src/amount.rs#L272-L299

methods in fungibles are no similar to multicurrency. pallet_assets has the ReservableCurrency as a type in its config:

/// The currency mechanism.
type Currency: ReservableCurrency<Self::AccountId>;

While in orml_tokens we can access as a struct:

impl<T, GetCurrencyId> PalletReservableCurrency<T::AccountId> for CurrencyAdapter<T, GetCurrencyId>

meaning the operation is both different.

I was thinking of creating a trait to cover all the differences between pallet_assets and ormls:

pub trait CurrencyExt<T:Config> {
    fn get_native_currency_id<T: Config>() -> CurrencyIdOf<T>;
    fn get_free_balance<T:Config>(currency_id: CurrencyIdOf<T>,
                                  account: &AccountIdOf<T>) -> Amount<T>;
    fn get_reserved_balance<T: Config>(
        currency_id: CurrencyIdOf<T>,
        account: &AccountIdOf<T>,
    ) -> Amount<T>;
}

So the user can fill in the body for pallet_assets. Do you think it's the best approach?

Also, I'm not quite sure how to rewrite the fn get_native_curency_id(); it could be a constant of pallet_currency as I don't see a similarity in pallet_assets. (But then this will become a duplicate, when using orml_currencies).

Need opinion.

[TorstenStueber]

@b-yap I think we might run into a dead end here. Originally I expressed that I would like Spacewalk to depend on Fungibles instead of MultiCurrency. However, that was at a time when I was not fully understanding the main differences between pallet-assets and orml-tokens yet.

The main problem we face is that pallet-assets did so far have no mechanism to reserve balances. For that reason this mechanism has also not been part of the Fungibles trait. The fact that pallet_assets has a reference to ReservableCurrency in its config is unrelated in my opinion – this is just use to reserve native balance if someone wants to register an asset. It cannot be used to reserve balances of any arbitrary asset.

But we need to be able to reserve balances because of the collateral.

There is a new addition to the family of Fungibles traits: namely holds and freezes. This has been merged in this PR. I got aware when Gav reported this on Element.

This PR only found its way into release of Polkadot version 0.9.42, which just got released ... 1 hour ago.

Furthermore, it seems like these traits are just defined but not implemented yet in pallet-assets, let alone in orml-tokens.

I think it might be best to put this issue on hold until orml-tokens implements these new traits.

[TorstenStueber]

I opened a ticket in orml: https://github.com/open-web3-stack/open-runtime-module-library/issues/911

[ashneverdawn]

So, it's been a while since I looked at this, but here's my thought:

I think the intention here really is to allow the user of the spacewalk library to use whatever currency solution they like. With that in mind, we ideally only need to provide a trait specifying what spacewalk actually needs, no more, no less. Then they can impl the trait on whatever currency solution they have and just have it call their underlying methods.

Spacewalk doesn't need to know about whether pallet_assets or orml or some other unique solution is being used. Whatever it is, it is on the library user's end that it would be made to satisfy our trait.

Our trait might even just be a subset of the current orml and that would be fine.

And, we can of course provide examples of impl our trait for orml and for pallet_assets. The pallet_assets example doesn't have to exist yet if it doesn't have features we need for spacewalk to work.

[ebma]

@ashneverdawn has a fair point. But then it's not worth doing this right now and we can think about it again once holds and freezes are implemented to pallet-assets as @TorstenStueber mentioned. Otherwise we over-engineer this although it's only really usable with orml-tokens right now anyways.

[TorstenStueber]

Sure, however, it would be just wrong to define our own trait in my opinion. Instead we should take any of the established standard traits, and that would only be the Fungibles family or the MultiCurrency family. That's why we defined this ticket in the first place.

We decided a long time ago to go with Fungibles because both pallet-assets as well as orml-tokens implements it and that way every parachain that has multi currency abilities would be able to use Spacewalk.

On the other hand: Gav expressed to me that there is no intention to implement the new Fungibles traits in pallet-assets.

For that reason we probably will only ever be able to support chains that implement orml-tokens (and that only if these traits get implemented there).

Regarding all of this, I propose now that we should actually close this ticket and just continue to use MultiCurrency. Sorry for all that extra effort.

[ebma]

I don't mind sticking to MultiCurrency. As I said, there is no need to over-engineer it right now. If we see the need to open up our currency to other pallets, we can still do it once required.