fix: multilocation type in pallets/foreign-assets #1408
Conversation
| 'XcmV3MultiLocation', | ||
| JSON.parse(foreignAssetMultiLocationStr), | ||
| ); | ||
| const XcmV3MultiLocation = ['StagingXcmV3MultiLocation', 'XcmV3MultiLocation']; |
There was a problem hiding this comment.
do we need to create the type from hardcoded options? Bear in mind this will be an issue soon when XCM v4 is released and XCM types are renamed.
There was a problem hiding this comment.
actually, this should be already the case if foreign assets in Rococo Asset Hub are queried.
There was a problem hiding this comment.
- You mean retrieve dynamically the
XCM Multilocationrelated types, check what is available and try the ones that should work right ? I haven't tried to create types this way but one thing that comes into my mind is the potential impact in theresponse time. 🤔 If there is such an implementation, I am guessing it will take longer to process the request (go through these steps) in order to return the expected result. - I just tested against Rococo Asset Hub and it returns the foreign assets correctly.
There was a problem hiding this comment.
- You are right, that solution would be an overkill, but we just need keep an eye for this changes.
- indeed, this makes sense for now, because we are talking about the storage types not the XCM types from the messages. But if Asset hub follows XCM v4 typing, then this will break again, just have it in mind.
| } catch (e) { | ||
| throw new InternalServerError(`Failed to create MultiLocation type for Foreign Asset`); | ||
| } | ||
| } | ||
|
|
||
| const assetMetadata = await api.query.foreignAssets.metadata<AssetMetadata>(foreignAssetMultiLocation); |
There was a problem hiding this comment.
Is it possible to just put the multilocation in the query as an Object or string and let the API take care of the type creation.
| 'XcmV3MultiLocation', | ||
| JSON.parse(foreignAssetMultiLocationStr), | ||
| ); | ||
| const XcmV3MultiLocation = ['StagingXcmV3MultiLocation', 'XcmV3MultiLocation']; |
There was a problem hiding this comment.
We can get rid of all of this, the try catches, and the xcm multilocation array, and just pass in an object of the multilocation directly to the query. Given this comment we dont want to be using createType directly anymore unless its testing, and we want to avoid using a nested try catch at all costs.
Description of the issue
The endpoint
pallets/foreign-assetsthrows the following error:Root Cause
The error is thrown when it tries to create the
XcmV3MultiLocationtype in this line.Suggested Solution
First try to create the
StagingXcmV3MultiLocationtype and if it is not successful try theXcmV3MultiLocationtype.Testing
Tested the endpoint by connecting to Polkadot/Kusama/Rococo Asset Hub