-
Notifications
You must be signed in to change notification settings - Fork 363
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: ChargeAssetTxPayment to work with Asset Conversion on Westend's and Kusama's Asset Hubs #5752
Conversation
I can confirm this fixes the decoding issue we've been seeing with the assetId as a multilocation. |
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.
Seems sane, thank you.
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.
Some small nigglies identified by CI, see https://github.com/polkadot-js/api/actions/runs/6870177012/job/18703510196?pr=5752#step:5:1294
It basically just needs a default type definition for the added type - generally we probably want to point this to the latest definition, which also means that the higher-version specs don't need the override.
So basically -
- add a default to point to
Option<MultiLocation>
- keep the override for lower versions (as required)
- remove the override for current versions (meaning we don't need to keep adding them into the future)
@jacogr I added the default types as you suggested and it solved the issues, but I also had to modify the Signer test to account for the assetId being a MultiLocation |
@bee344 All good, I appreciate your persistence :) |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Since the addition of the new
pallet-asset-conversion
toKusama Asset Hub
andWestend Asset Hub
, theChargeAssetTxPayment
mutated to accept aMultiLocation
as the ID of the asset used to pay the tx fees, which in turn broke the decoding of the blocks in which it was present. What this PR does is fix that by modifying the signed extension for the cases of KAH and WAH, by integrating an alias that modifies the SE to useAssetId
orMultiLocation
depending on the runtime version. At the same time, as the introduction of the pallet was at different times on KAH and WAH and was not added in Polkadot Asset Hub, it addstype
override files for each of them, and leaves thestatemint
one without change.It also modifies
SignerOptions
to accept anobject
for the SEassetId
, so that aMultiLocation
can be passed and the SE be used in a straightforward manner.Block retrieved with substrate-sidecar pre-fix:
Block retrieved with substrate-sidecar after fix:
Closes Issue 5750 and 5734 (probably).