search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.78k stars 640 forks source link

Better delivery fees: DepositFee #3434

Open franciscoaguirre opened 6 months ago

franciscoaguirre commented 6 months ago

As you all know, there have been a lot of XCM related issues in Kusama lately. This is because of the new delivery fees not being integrated in the best way to the executor.

There are three problems I can identify:

  1. Getting delivery fees from holding during execution
  2. Estimating delivery fees
  3. Fixing delivery fees for multi-hop messages

For 1. we need a system that's similar to BuyExecution but for delivery fees. We could have an instruction BuyTransport that works in a similar way. You pay once for all delivery fees and can be certain the rest of the execution will succeed.

Something like BuyTransport would work, but I'd like to take this opportunity to not add more must-use instructions that make XCM harder to use. It's already hard enough when people forget a BuyExecution instruction. I think it's worthwhile to pay for these fees transparently.

Different xcm senders can charge different prices, and we can only identify them via calling validate_send, which calls a tuple and each element either returns the price or fails and it tries the next one. These senders validate using the destination. So we need to know the destination of all messages that will be sent beforehand.

My proposal is to gather all destinations beforehand, and buy enough delivery fees for sending whenever we pay for them, be it on a custom instruction or on the first instruction that loads the holding register.

The size of the message is also important, so I suggest we use a worst-case message in case it doesn't increase costs too much.

I'll leave the other two for other issues.

L.E.: XCM RFC for this https://github.com/paritytech/xcm-format/pull/53

xlc commented 6 months ago

Related https://github.com/paritytech/polkadot-sdk/issues/690

From user point of view, the only thing they care is the total fee. They don't care if the fee is used for buy execution or buy transport. It shouldn't matter.

So my suggestion is make a fee register, rename BuyExecution to DepositFee and simply charge fees from the fee register. In future we can introduce more ways to charge fee and it wouldn't make any impact.

franciscoaguirre commented 6 months ago

I'm making a prototype in #3450, the idea is the following:

My only concern is the use of a barrier to gather the destinations of where the messages will be sent. We need this for estimating. In a way, this is similar to how the AllowTopLevelPaidExecutionFrom<T> barrier sets the actual weight of the message, to help BuyExecution further down the line. I appreciate any feedback on this.

acatangiu commented 6 months ago

I propose to split this in two separate tracks that we can tackle cleanly and independently, and when both are completed, the whole fees problem will be solved:

  1. A clean mechanism to charge fees (all types: execution, transport) from assets included in XCM program (e.g. assets being transferred), user must be able to provide an upper limit.
  2. A mechanism for finding the complete fees (including local, intermediary and destination chains) required for the successful execution of an XCM program.

1. charging fees

This will not attempt to estimate fees.

So my suggestion is make a fee register, rename BuyExecution to DepositFee and simply charge fees from the fee register. In future we can introduce more ways to charge fee and it wouldn't make any impact.

This ^

In a new XCM version we can implement new DepositFee(fees: Asset) instruction (also deprecate/remove jit_withdraw mechanism). Any unspent fees at the end of the execution are trapped for future claim back.

This means we can implement it and be useful for users even before/without having 2. As long as users overestimate fees, their program will not fail, and excess/unspent can be claimed back.

We also need to fix assets trapping and claiming

  1. we need https://github.com/paritytech/xcm-format/blob/master/proposals/0038-execute-with-origin.md for actually trapping assets on remote chains (right now most programs do ClearOrigin which kills the asset trap as well)
  2. we need https://github.com/paritytech/xcm-format/blob/master/proposals/0037-custom-asset-claimer.md for providing better UX on claiming back trapped assets

2. Fees discovery

This is being discussed in https://github.com/paritytech/polkadot-sdk/issues/690

Current proposal is to provide runtime apis (callable from on-chain or off-chain) to effectively "dry-run" an XCM program, providing info on things like:

The same runtime API can then be called off-chain on [intermediary, +] destination chains recursively dry-running the forwarded XCMs from previous hop.

This way a wallet/dapp can dry-run the e2e path and get fees for each step.

This runtime API can be later genericized/replaced with a new XCQ mechanism to get the same info (and more) using generic chain-agnostic queries (rather than chain-specific runtime api calls).

Further, ideas were also discussed for some mechanism to actively lock-in the fee-price, to avoid synchronization issues between querying and executing.

1 & 2 together

Once we have both, users can find out how much fees are (in the future even lock in that price for a number of blocks), then adjust values used in the executed XCMs' DepositFee instructions to the right values.

franciscoaguirre commented 6 months ago

I agree with the roadmap to fix all the fee issues. However, I'm hesitant to add a new version for XCM, since it would take some time. We still need to remove V2 and V4 was released very little ago.

I understand a lot of programs will be broken by this change. I can work on a V5 version but it'll take a little bit. If not many people are using/have migrated to V4 yet, we could do it there. It's not even released in Kusama yet.

franciscoaguirre commented 6 months ago

Nevermind, we should definitely do a new version for this