RFC: Better fee handling

[franciscoaguirre]

XCM already handles execution fees in an effective and efficient manner using the BuyExecution instruction.
However, other types of fees are not handled as effectively -- for example, delivery fees.
Fees exist that can't be measured using Weight -- as execution fees can -- so a new method should be thought up for those cases.
This RFC proposes making the fee handling system simpler and more general, by doing two things:

• Adding a fees register
• Replacing BuyExecution with a new instruction PayFees with new semantics

This new instruction only takes the amount of fees that the XCVM can use from the holding register.
The XCVM implementation is free to use these fees to pay for execution fees, transport fees, or any other type of fee that might be necessary.
This moves the specifics of fees further away from the XCM standard, and more into the actual underlying XCVM implementation, which is a good thing.

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/xcm-rfc-better-fee-handling/6547/1

[franciscoaguirre]

A PoC was done for Polkadot in paritytech/polkadot-sdk#3450.

[acatangiu]

I am supportive of this change 👍 looking forward to community feedback

[acatangiu]

When coupling this with a good fee discovery mechanism (being discussed in paritytech/polkadot-sdk#690), this concern is diminished. At least from users acting in good faith. We still need to consider if/how this can be exploited in bad faith.

[xlc]

yeah current trap asset implementation is flawed paritytech/polkadot-sdk#901

[franciscoaguirre]

It would make sense to not trap fees, or (fees + assets) if the amount is below a particular threshold. That way we prevent extra storage bloat.

[spazcoin]

Instead of trapping the asset, can you automatically RefundSurplus and DepositAsset back to the account being executed from, to reduce the likelihood that someone would forget to execute those operations and slowly lose assets over time?

[pandres95]

Instead of trapping the asset, can you automatically RefundSurplus and DepositAsset back to the account being executed from, to reduce the likelihood that someone would forget to execute those operations and slowly lose assets over time?

I thought about this once, but this assumes an explicit implementation more related to XCVM rather than XCM (for example, consider cases where more than one asset is being withdrawn, how is this automatically handled by the transaction format without enforcing an XCVM specific behaviour?).

I think that part of the design is okay as it is now.

That said, yeah, the assets trap design can be improved.

[xlc]

I guess RefundSurplus will need a revisit as well

[xlc]

I also don't like trapping asset. With the current model, we almost always needs a RefundSurplus + DepositAsset at end of the XCM to avoid trapped asset. Can we do it better?

I see a few options:

• Make PayFees also takes another optional refund address and automatically refund unused fees to it at end of the execution
• Have a version of DepositAsset that deposit assets from all the asset register including fees and holdings
• A SetTrapAssetDestination instruction that automatically deposit trapped asset into an account, instead of actually trap it
• No change. Just inform devs that it is recommended to always have RefundSurplus + DepositAsset at end of the XCM to avoid trapped asset.

[franciscoaguirre]

I agree it's not the best to always require RefundSurplus + DepositAsset.

I like the idea of depositing to some account instead of trapping the assets. At least letting the user specify that behaviour.

For example, SetAssetClaimer is already approved, which lets the user specify who can claim trapped assets.
We could have something similar that allows the user to specify they want to deposit leftover assets to a particular location instead of trapping them.
However, this would just reduce the two instructions (RefundSurplus + DepositAsset) to one (something like SetLeftoverFeesDestination).
Not sure how much more useful that is.

We could make the default behaviour be to deposit the leftover fees to the Location specified by origin, given that we expect them to always be a little bit more than needed. If the user wants to specify the location for said leftover fees (say, an account they own in the local system that's not the sovereign account), they can specify it.

I think this would change the least assumptions about XCM programs. What do you think?

[acatangiu]

I haven't properly thought it through, so I won't offer an opinion on what is best option, but I want to call out early that we should design this (and further changes) to be easily usable in multi-hops XCM programs.

E.g. going through remote reserve where the user might not have an account, or going over a bridge where delivery fees need to be paid at both intermediary bridge-hubs, etc.

[xlc]

One benefit of SetXXX is that we call it at the beginning of the execution and it will be executed no matter what. On the other hand, the RefundSurplus at end may not be skipped because errors. We do have instructions to handle errors but then it maybe a bit more complicated? Actually we don't really have a best practice about how to write error-proof XCM and someone need to spend a bit of time to think about all the situations and come up some good recommendations. e.g. always have SetAppendix to execute RefundSurplus + DepositAsset to ensure no trapped asset no matter what.

Some disadvantage of SetXXX instructions is that the implementation will be slightly more complicated as there are more state to remember. But I think that's not a real issue.

[franciscoaguirre]

Yeah the SetX instruction would need a new register holding a Location to where you want to deposit leftover fees. I think it's a good idea to have it.

Also, I agree best practices are definitely needed.

[albertov19]

I was never a fan of asset traps because it is not trivial to recover traps, etc. - I like having a field in PayFees where assets left in the holding register are sent to if program execution fails.

Even by default, I think the address should be the sovereign account of the origin chain. Therefore, it is up to the governance system of such a chain to help recover the assets.

@xlc - you can always put the RefundSurplus + DepositAsset within a SetAppendix so that it is "ensured" that they get executed if program execution fails. That is what we do with the XCM-Transactor we use at Moonbeam to do remote transacts (although we make it optional because before the cost of adding those extra instructions was "high")

[acatangiu]

I was never a fan of asset traps because it is not trivial to recover traps, etc. - I like having a field in PayFees where assets left in the holding register are sent to if program execution fails.

With the upcoming SetAssetClaimer, you could achieve exactly that without having to "bake it in" the PayFees instruction.

Redesigning the asset trapping mechanism is also something on the TODO list, with ideas like depositing directly to sovereign accounts (custom account if set by SetAssetClaimer or the sovereign account of the origin chain by default). But that should be discussed in a dedicated RFC imo.

[xlc]

We had another XCM transfer issue on Acala related to Acala specific fee handling that could be addressed by this new fee mechanism by allowing the parachain to charge additional fees on top of the existing XCM fee types.

[acatangiu]

We had another XCM transfer issue on Acala related to Acala specific fee handling that could be addressed by this new fee mechanism by allowing the parachain to charge additional fees on top of the existing XCM fee types.

AFAICT the current proposal would allow that if you're implementing custom executor with custom PayFees execution, right?

What kind of "additional fees" should we be thinking about? Is it something we should just add support for in existing executor (some customizable extra fee types)?
Or is handling them with custom executor better?

[franciscoaguirre]

The only config item we have for fees is the Trader, but that's tightly coupled to weight. We could provide a new config item to make it easier to define custom fees. That's regardless of the standard

[xlc]

For the Acala specific use case, we charge an additional storage deposit for EVM calls. In the case that the EVM call is triggered by XCM, we found we are in a poor situation that we don't have a good way to charge this deposit.

We cannot charge it from the sovereign account, because otherwise it will be an attack vector to drain those accounts.
We cannot charge it using the XCM fee mechanism because there is no such mechanism.
The only thing we can do is charge from the receiving account, and that's a bad idea as well.

[franciscoaguirre]

What I'm thinking we can do after this new fees register, is having a custom CallDispatcher that checks if the call inside Transact is an EVM call and uses fees from the fees register to pay for the storage deposit.
That would only need access to the fees register from the CallDispatcher, and would allow everyone to create custom fees.
This is very hacky and we could even do it if we give it access to the holding register right now.

The good approach I can think of is adding a config item that can be a tuple of structs that have access to the fees register. Then the implementation of PayFees will loop through them and charge all the fees. We could even add a log for each of them so the user knows why it was charged that.
This would need to be accompanied by a good way of getting these fees so UIs can estimate them. We don't have anything that would fit this in the XCM fee estimation API. We could add a call for additional fees.

Just brainstorming here. The additional fees scenario is a different one from the main one this RFC is trying to solve though.

[xlc]

having a custom CallDispatcher that checks if the call inside Transact is an EVM call

that wouldn’t be enough. ERC20 tokens are first class citizens at Acala. ie we allow transfer them via XCM using deposit/withdraw mechanism. so it is possible to invoke EVM without transact

We currently have a global variable (implemented using a storage entry) to provide additional context to EVM so it knows where to charge the storage deposit. A fee register if implemented properly, will allow us to charge deposit from it. And we can simply make deposit/withdraw action to fail if unable to charge enough deposits.

[mrshiposha]

The interface of the PayFees instruction is great!
It allows for accumulating all the fee assets to cover every fee type.

I'd like to discuss the usage of this instruction by clients/users.
It would be great if users did not bother themselves with different "fee types" and did not have to worry about what currencies they should pay for each fee type.

And it would be awesome to be able to do something like this:

1. Suppose a user holds two currencies (CURRENCY_A and CURRENCY_B). Let the user have AMOUNT_A and AMOUNT_B of the corresponding currencies.
2. Imagine the user wants to execute an XCM program, but neither AMOUNT_A nor AMOUNT_B is enough to cover the fees.
3. But suppose, using the PayFees instruction, the user can supply both AMOUNT_A and AMOUNT_B, and the XCM executor will handle the rest. The user doesn't know for what "fee type" and in what proportion their funds will be used. The user only cares about whether it is enough to execute the program or not. The user isn't interested in the details.

To achieve this, we could introduce an abstract asset type that the XCM Executor could use internally (we can call them "XCM credits"). I'm not sure if it should be part of the spec; it seems more like an implementation detail. Still, I believe it is worth discussing here nonetheless.

The "XCM credits" may be used to cover:

1. The execution fees (with an appropriate conversion from Weight to these Credits)
2. The delivery fees where the amount of these Credits will depend on the congestion of the HRMP channel
3. Any other fee type

A concrete blockchain could define the appropriate two-way conversion between its acceptable payment assets and these Credits.

So, the PayFees instruction / fees register semantics could be defined in more detail:

1. This instruction populates the fees register
2. The fees register contains the list of (asset_id, amount_of_asset, corresponding_xcm_credits)
3. When the XCM implementation wants to deduct the Credits from the fees register, it will deduct them in order of this list (i.e., if the Credits obtained from one asset type are drained, the next one is used).
4. If there are any leftovers with non-zero corresponding_xcm_credits in the fees register, the remaining Credits should be converted back into their corresponding assets. These assets might be dealt with as discussed in the comment above: RFC: Better fee handling #53 (comment).

The XCM Fee Payment Runtime API could also be modified accordingly so clients can:

1. convert programs into Credits
2. convert a list of assets into the total number of credits they represent

[xlc]

While there are some use cases of accepting multiple tokens for fee payments, I feel it is an edge case that can be easily avoided on the frontend side and the additional complicity does not worth the trade-off.

The frontend should just figure out what asset should be used for XCM fee and optionally provide a way for user to change that. If no single asset is enough, display not enough fee is a reasonable outcome.

[mrshiposha]

@xlc Even if we don't want to support payment in multiple user-supplied currencies, there is another reason to introduce abstract Credits (by the way, in the simplest case, they can be defined as equivalent to the chain's native token).

I am worried that the execution fee and the delivery fee (and any potential custom fee types) may currently require different tokens for payment on a single chain. With the BuyExecution instruction, users were able to select the token to cover the execution fees at least. With PayFees, the payment asset for any fee type is chain-defined.

The idea is to introduce an abstract asset to express any fee type so any acceptable user-supplied asset (singular) will cover all the fee types on a single chain (any acceptable asset can be converted to Credits). Thus, the user or a JS client won't need to think about different fee types and different required assets for each one within a single chain. They would be able to select a singular token to cover everything.

Payment in multiple currencies becomes possible with this approach (and the XCM executor can handle it without worrying the chain developers), but it is optional.

[xlc]

I don’t think that’s a real issue as the xcm on different chain are different and therefore it is possible to specify what assets for fee payment on each chain. It will be silly for a chain to require multiple assets for fee payments. Besides, the credit system will make refund fee lot more complicated.

[mrshiposha]

I'm worried because AssetHub does this. You can pay for the execution using USDT, but you also must have enough DOT to cover the delivery fees. It is a small fee, but nonetheless -- you pay for the execution in USDT and delivery in DOT.

• xcmp-queue uses PriceForSiblingParachainDelivery
• delivery fee asset id == DOT
• When I send 1 USDT from Unique to Astar, then I must supply not only USDT for Asset Hub's execution fees but also some DOT (which is impossible since DOT's and USDT's reserve locations differ). See in the block, the assets were trapped. The error is NotHoldingFees, but 1 USDT should be enough to cover the execution fees. I suspect it failed here when it tries to deduct the delivery fees in DOT

[acatangiu]

I'm worried because AssetHub does this. You can pay for the execution using USDT, but you also must have enough DOT to cover the delivery fees. It is a small fee, but nonetheless -- you pay for the execution in USDT and delivery in DOT.

* [xcmp-queue uses PriceForSiblingParachainDelivery](https://github.com/polkadot-fellows/runtimes/blob/c0e66c3e121cc5da7d59415015019c3bb0ec6581/system-parachains/asset-hubs/asset-hub-polkadot/src/lib.rs#L581-L586)

* [delivery fee asset id == DOT](https://github.com/polkadot-fellows/runtimes/blob/c0e66c3e121cc5da7d59415015019c3bb0ec6581/system-parachains/asset-hubs/asset-hub-polkadot/src/lib.rs#L575)

* When I send 1 USDT from Unique to Astar, then I must supply not only USDT for Asset Hub's execution fees but also some DOT (which is impossible since DOT's and USDT's reserve locations differ). See [in the block](https://assethub-polkadot.subscan.io/block/0xe45496a3cc1e472282df659d254c17e31fc87ad76dac5bb424b7ef801f6f3a64?tab=event), the assets were trapped. The error is `NotHoldingFees`, but 1 USDT should be enough to cover the execution fees. I suspect it failed [here](https://github.com/paritytech/polkadot-sdk/blob/ecfcb42dfb419ebc7fee0793dbd1f9dbf0089883/polkadot/xcm/xcm-executor/src/lib.rs#L1094) when it tries to [deduct the delivery fees](https://github.com/paritytech/polkadot-sdk/blob/ecfcb42dfb419ebc7fee0793dbd1f9dbf0089883/polkadot/xcm/xcm-executor/src/lib.rs#L457-L458)

Going forward I hope to hear from the ecosystem about these kinds of problems and try and fix them instead of building complicated machinery either on core protocol or upper layers to handle them.

What you describe is a good example of this: a problem we need to fix (paritytech/polkadot-sdk#3958) rather than build more complexity around it to work around.

[mrshiposha]

If taking several assets for different fee types on a single chain is always considered a problem that the chain's team should fix, I'm okay with that :)

I'd just be happy to see an interface that makes it easy "to do fee-related things right" and harder "to do wrong". However, the credit system would be an unpleasant migration for chains with big XCM configs (such as Acala). Also, an audit of the XCM executor would certainly be required, even if the actual implementation is relatively simple. So, yeah, the credit system requires much stronger reasons to be introduced.

Speaking of interface, if we agree that only one asset should be used to cover all the fees on a single chain, should the PayFees take multiple assets (as it currently does)? Is there a reason for that? Can we make it take just an Asset?

[xlc]

While the chain should only require a single asset for fee payment, the chain may also accept multiple assets for fee payment purpose. In the example of Acala, we accept multiple tokens for XCM fee payment including ACA, DOT, LDOT, etc. User only need to supply one of those accepted token and able to perform XCM execution on Acala.

[mrshiposha]

the chain may also accept multiple assets for fee payment purpose

Of course. It is not what I'm asking.

The Asset type definition is:

pub struct Asset {
	/// The overall asset identity (aka *class*, in the case of a non-fungible).
	pub id: AssetId,
	/// The fungibility of the asset, which contains either the amount (in the case of a fungible
	/// asset) or the *instance ID*, the secondary asset identifier.
	pub fun: Fungibility,
}

At the same time, Assets (the type the PayFee uses currently) has the following definition

pub struct Assets(Vec<Asset>);

The current BuyExecution instruction takes an Asset, not Assets. But PayFees takes Assets. Why?

If the chain should only require a single asset for fee payment, then why does PayFees allow a user to supply a vec of the different assets? For instance, vec![(ACA, 10), (DOT, 2), (LDOT, 3)]).
PayFees instruction moves all the assets in the list (of different types) from the Holding Register to the fees register.

If we make PayFees accept the Asset type, the user is still capable of paying in different asset types, but only a single asset will be used in a single XCM program (just like it is with BuyExecution, but this time different fee types will be covered with this asset)

[franciscoaguirre]

But PayFees takes Assets. Why?**

I wanted to make it more generic, maybe you don't have enough of one asset and want to pay with a combination of assets. I agree though that this is too complicated for something that should be simple, like fees. I'll revert it to take only one asset.

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/2024-04-23-technical-fellowship-opendev-call/7592/1

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/xcm-user-and-developer-experience-improvements/4511/21

[xlc]

what's the next step?

[franciscoaguirre]

The next steps are finishing up the prototype, getting more RFCs for v5 and creating v5

[franciscoaguirre]

I've been busy with other tasks, but I can start with v5 in some capacity

[franciscoaguirre]

Closing in favor of polkadot-fellows/RFCs#105