Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Withdraw Unknown Assets Exhausts Block Weight #7182

Closed
albertov19 opened this issue May 5, 2023 · 6 comments · Fixed by #7201
Closed

Withdraw Unknown Assets Exhausts Block Weight #7182

albertov19 opened this issue May 5, 2023 · 6 comments · Fixed by #7201

Comments

@albertov19
Copy link

Currently, in Westmint based relay chains, if you include a WithdrawAsset instruction you get Invalid Transaction: Transaction would exhaust the block limits. This is because the Weigher forcefully assigns the MAX if the asset is unknown:

AssetTypes::Balances => balances_weight,
AssetTypes::Unknown => Weight::MAX,

This completely blocks the possibility of controlling remote origins from the relay chain as you can't send XCM messages that withdraw other chains assets from remote origin accounts. One solution is to use SUDO and utility.withOrigin but that limits the applicability as you can only do it in setups where SUDO is available and known.

Why is the XCM message being weighted by xcm.Send? Is that really necessary as the XCM is not being locally executed? Another option would be to provide a field for refTime and proofSize so it can be manually set and is not forced to be MAX

@bkchr
Copy link
Member

bkchr commented May 5, 2023

CC @KiChjang

@KiChjang
Copy link
Contributor

KiChjang commented May 8, 2023

Why is the XCM message being weighted by xcm.Send? Is that really necessary as the XCM is not being locally executed?

We need to know how much weight the destination chain will use to execute the XCM, in order to provide a proper amount to BuyExecution. Obviously the ideal way would be to weigh the XCM using the destination chain's weigher, but we currently don't have any sort of mechanism to do so, hence we're using the local weigher as an approximation.

Moreover, the API of the XcmWeightInfo doesn't contain the destination of the XCM, so we couldn't make a scheme where we give a user-defined weight for XCMs intended for foreign destination and retain the current logic for locally-executed XCMs. There is also no way of providing a user-defined weight to the API either; if you have any ideas on how to do so, I'm all ears.

@albertov19
Copy link
Author

Thanks for the reply @KiChjang - But can you just provide the weight in the BuyExecution instruction itself?

If you use Unlimited the destination chain will just calculate the amount of weight you can get with the fungible amount you provide.

send_xcm does not enforce any BuyExecution (here - The user using the exrinsic needs to manually set and configure the instruction.

So this actually breaks the send_xcm pallet for remote origin control.

@KiChjang
Copy link
Contributor

KiChjang commented May 8, 2023

Wait sorry, the entire flow is actually weighed twice for 2 different purposes -- once for BuyExecution, and once for the FRAME extrinsic payment. The former is indeed covered by simply specifying Unlimited in BuyExecution, but the latter isn't. Looking at it, it would actually seem to me that we're paying for execution twice as a result.

Let me check this first and I'll get back to you to see whether this is indeed the case.

@KiChjang
Copy link
Contributor

KiChjang commented May 8, 2023

Okay, so I've figured out now that we indeed are overpaying for the weight. The send extrinsic should only just be T::WeightInfo::send() and shouldn't be trying to weigh the XCM; all of the XCM execution weight is handled by the destination, and the extrinsic only cares about the weight of sending the XCM, not executing it.

@albertov19
Copy link
Author

Amazing! Thanks @KiChjang and @franciscoaguirre for the quick turnaround

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 a pull request may close this issue.

3 participants