-
Notifications
You must be signed in to change notification settings - Fork 700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix XcmPaymentApi::query_weight_to_asset_fee
version conversion
#6459
base: master
Are you sure you want to change the base?
Conversation
bot fmt |
@franciscoaguirre https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7748401 was started for your command Comment |
@franciscoaguirre Command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgmt modulo @franciscoaguirre could you please check #6467 - it could simplify
cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
/cmd fmt |
Command "fmt" has started 🚀 See logs here |
Command "fmt" has finished ✅ See logs here |
@bkontur I deduplicated the test and extended it to call |
/cmd help |
Command "help" has started 🚀 See logs here |
Command "help" has failed ❌! See logs here |
/cmd --help |
Command help:
|
very nice, I am just thinking now, that we could/should split
or something like that, because not every runtime has the pools, e.g. coretime/people/relaychains... |
Right now it's in assets/test-utils/. If I divide it in two I can put them in test-utils and use the pools test only in asset hubs |
The
query_weight_to_asset_fee
function was trying to convert versions by usingtry_as
, this function doesn't convert from a versioned to a concrete type.This would cause all calls with a lower version to fail.
The correct function to use is the good old try_into.
Now those calls work :)