Skip to content
This repository has been archived by the owner on Jul 7, 2021. It is now read-only.

Allow Paying of Transactions via Assets #51

Closed
wants to merge 7 commits into from

Conversation

apopiak
Copy link
Contributor

@apopiak apopiak commented May 6, 2021

This PR adds a new pallet that allows for tx fee payment in either the native currency or an asset.
The asset fee is determined from the native fee via a newly introduced conversion function in the asset pallet (that uses the ratio between the minimum balances).

related to #5

apopiak added 4 commits May 4, 2021 11:17

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
AssetBalanceOf<T>: FixedPointOperand,
{
fn to_asset_balance(balance: BalanceOf<T>, asset_id: AssetIdOf<T>) -> Result<AssetBalanceOf<T>, ()> {
let res = pallet_assets::BalanceToAssetBalance::<T, BalanceOf<T>, <T as pallet_balances::Config>::ExistentialDeposit>::to_asset_balance(balance, asset_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Bird's eye view, I'd add type BalanceToAsset: Convert<Balance, Asset> (or a custom trait) to the config of asset-tx-payment rather than relying on pallet-assets directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate what this pallet will do?

I know that the goal is to pay tx-fees in an asset, but why is there a conversion of a balance to asset? if anythign I'd expect to see the opposite: we need to convert the asset to balance, and then relay it to the normal tx-payment that uses a balance, or somethign along those lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you have it backwards.
We already have fee calculation logic that determines the fee in terms of the native currency. What we now want to allow is paying tx fees with an asset, so what we need to do is convert the fee from native to asset and then charge the account sending the transaction.
Most of that should already be implemented in this pallet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we tweak the current pallet-transaction-payment to:

instead of type Currency: Currency<_>, work with a more generalized trait that is implemented by both pallet-assets and pallet-balances. Does such a thing exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But we need custom data in the transaction: the asset id

Comment on lines +183 to +185
<T::Fungibles as Balanced<T::AccountId>>::withdraw(asset_id, who, converted_fee)
.map(|i| (fee, InitialPayment::Asset(i)))
.map_err(|_| -> TransactionValidityError { InvalidTransaction::Payment.into() })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kianenigma (initial) fee payment happens here in case an asset was specified.

use frame_system::pallet_prelude::*;

#[pallet::config]
pub trait Config: frame_system::Config + pallet_transaction_payment::Config + pallet_balances::Config + pallet_authorship::Config + pallet_assets::Config {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub trait Config: frame_system::Config + pallet_transaction_payment::Config + pallet_balances::Config + pallet_authorship::Config + pallet_assets::Config {
pub trait Config: frame_system::Config + pallet_transaction_payment::Config + pallet_assets::Config {

pfff, do we need all of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure we will need them in the end, just added them for easy hacking.
Authorship will probably be necessary because we access the block author to pay fees to them.
Balances we can most likely remove.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a clear way to listen to authorship events: you implement pallet_authorship::EventHandler and add yourself to the list of authorship event handlers.

Still not pretty as you need to import pallet_authorship, but your config need not be a sub-trait of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And then I store the author? Because I need access to the author on when the transaction payment is triggered.

Copy link
Contributor

Choose a reason for hiding this comment

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

I won't block on this, but the clean thing to do is to, similar to CurrencyAdapter, just accept a T: OnUnBalanced and then give it to a top level struct DealWithFees that can read the author or whatever.

@apopiak
Copy link
Contributor Author

apopiak commented Jun 10, 2021

superseded by paritytech/cumulus#488

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants