XCM: fix for Transact v5->v4 decoding #6609
Conversation
bkontur
added
R0-silent
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
| let require_weight_at_most = call.take_decoded()?.get_dispatch_info().call_weight; | ||
| let require_weight_at_most = call.take_decoded() | ||
| .map(|c| c.get_dispatch_info().call_weight) | ||
| .unwrap_or(Weight::MAX); |
There was a problem hiding this comment.
@franciscoaguirre I am not sure if MAX is ok, we could also go with MAX/2, MAX/4 ..., previously we had here:
// We use a big weight here but not `Weight::MAX` as a best effort.
const DEFAULT_WEIGHT_FOR_TRANSACT_CONVERSION: Weight = Weight::from_parts(10_000_000_000, 1_000_000);
at least MAX is working for #6585
I tested this with chopstics for https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fwestend-rpc.polkadot.io#/explorer/query/0x01cd07a4ede58f34d76422febdf1adc28669c1a2ed5fa5ccaa7d15a7d1bfb0a5
Probably Weight::MAX shouldn't be used, I am no sure, because of chains which have just XCMv4 processing:
https://github.com/paritytech/polkadot-sdk/blob/stable2409/polkadot/xcm/xcm-executor/src/lib.rs#L862-L871
https://github.com/paritytech/polkadot-sdk/blob/stable2409/polkadot/xcm/xcm-builder/src/weight.rs#L65
There was a problem hiding this comment.
if call.take_decoded()?.get_dispatch_info() only fails for calls of type (), then using MAX should be ok, in practice there should be no Transact instructions for any Xcm<()>. Any real Transact will have a call type different than (), no?
There was a problem hiding this comment.
The issue is when we send it we'll end up telling the receiver to use MAX. I think MAX divided by some factor should be good. Or looking at the on-chain data and picking a value that is big enough for most transacts
There was a problem hiding this comment.
The problem is that the weight here in require_weight_at_most is what is actually initially charged for execution fees on older chains, so dryrunning an xcmv5 program says use these fees, then sending that program to an xcmv4 chain will have the call fail with not enough fees
(someone needs to double check this and confirm in code)
There was a problem hiding this comment.
Yeah we should go for a more backwards compatible solution
|
/cmd fmt |
|
Command "fmt" has started 🚀 See logs here |
|
Command "fmt" has finished ✅ See logs here |
| let require_weight_at_most = call.take_decoded()?.get_dispatch_info().call_weight; | ||
| let require_weight_at_most = call.take_decoded() | ||
| .map(|c| c.get_dispatch_info().call_weight) | ||
| .unwrap_or(Weight::MAX); |
There was a problem hiding this comment.
if call.take_decoded()?.get_dispatch_info() only fails for calls of type (), then using MAX should be ok, in practice there should be no Transact instructions for any Xcm<()>. Any real Transact will have a call type different than (), no?
Yes, agree, it just allows to pass on the sending side (I tested with chopstics), at least we know that this conversion is the issue. |
Closes: #6585
Relates to: #6228