-
Notifications
You must be signed in to change notification settings - Fork 7
Allow Paying of Transactions via Assets #51
Conversation
pallets/asset-tx-payment/src/lib.rs
Outdated
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) |
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.
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.
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.
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.
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.
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.
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.
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?
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.
But we need custom data in the transaction: the asset id
<T::Fungibles as Balanced<T::AccountId>>::withdraw(asset_id, who, converted_fee) | ||
.map(|i| (fee, InitialPayment::Asset(i))) | ||
.map_err(|_| -> TransactionValidityError { InvalidTransaction::Payment.into() }) |
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.
@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 { |
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.
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?
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.
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.
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.
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.
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.
And then I store the author? Because I need access to the author on when the transaction payment is triggered.
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 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.
superseded by paritytech/cumulus#488 |
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