Pay delivery fees from the assets available during execution

[franciscoaguirre]

Implements XCM RFC polkadot-fellows/xcm-format#53

Related: #3434

The problem this PR aims to address is the lack of functionality to pay for delivery fees directly with assets available during execution.
The way it solves it is by hijacking the BuyExecution instruction. After it has paid for execution, the unspent assets are used to pay for the delivery fees.

We put all unspent assets after paying for execution fees into a new delivery_fees register.
We could estimate how much to put and return the rest to the holding register, but estimating how much is needed is very complicated -- see commit history for how/why.

Because we just put all the assets into this register, some programs would probably need to change.
Before this change, there's no issue with users writing:

let xcm = Xcm(vec![
  WithdrawAsset(vec![asset]),
  BuyExecution { fees: asset, .. },
  DepositAsset { assets: asset.into(), .. },
]);

Now, this wouldn't deposit anything, since we take everything for fees.

Don't worry about the xcm simulator stuff, it's just to test this makes sense. I'll do better tests as I advance on this.

This is still a work-in-progress, but I'd appreciate any feedback about the approach, optimization concerns, security concerns, anything.

[xlc]

I just don't like it need to estimate the worst case fee first and then calculate the fee one more time later to actually charge the fee

[franciscoaguirre]

I agree that's not the best. I needed a way to know how much to take from the unspent assets used in BuyExecution.

We could just charge the worst case. That would let us only calculate it once, but it might be a lot of overpaying. I'd need to do some benchmarking to see how much more expensive that would be.

If people specified a specific amount for fees then it wouldn't be a problem either. However, if they specify all of the funds we could be keeping all their assets simply for fee payment.

We also need to estimate the delivery fee with the function since it changes overtime.

[xlc]

can we just have a fee register?

[franciscoaguirre]

But we do in this case. I can just put everything in unspent in the fee register if that's better

[franciscoaguirre]

It's clearly much simpler if we don't try to estimate the fees beforehand. I added that if there are assets left in the fee register after execution, then they're trapped. Seems like a simple solution that can work.

[acatangiu]

Because of this, we cannot make this change to BuyExecution - it will brutally or subtly break many XCM programs out there.
I don't see a way to fix this without a new XCM version.

See #3434 (comment)

[franciscoaguirre]

I'll instead add a new instruction DepositFee to make sure no existing programs are broken

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5333816

[xlc]

maybe we need to discuss the details with xcm rfc first? the original bug is introduced because lack of discussions and I am worried if we just keep writing code without discussion we will introduce more bugs. I don’t know about others but I had enough XCM bugs and tired of dealing with them.

[franciscoaguirre]

I agree we should have more discussions about this, I created an RFC for it and mentioned it in the polkadot forum: https://forum.polkadot.network/t/xcm-rfc-better-fee-handling/6547/1

[Polkadot-Forum]

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

https://forum.polkadot.network/t/parity-tech-update-for-march/7226/1

[franciscoaguirre]

Closing in favor of #5420