feat(extension-ui): extrinsic asset id

[ryanleecode]

Fixes: https://github.com/polkadot-js/extension/issues/1314

Adds assetId in the extrinsic UI. Assumes the assetId is an XCMV2 junction object with the junctions PalletInstance and GeneralIndex. Otherwise it won't render and neither will it crash because any errors are swallowed to null.

[ryanleecode]

lgtm!

Some things to note from when we spoke:

• works when sending an X2 pool asset location as the optional assetId (e.g. {parents: 0, interior:{X2:[{PalletInstance:50},{GeneralIndex:41}]}}
• supporting only theX2 junction may be limiting and could be expanded to support other location junctions (if it turns out to be necessary) but if this is out of scope for this fix another issue can be created
• PJS apps doesn't have the assetId appear in the extension UI due to the assetId value not being a part of the payload and would require additional changes

We should open an issue in polkadot-sdk to get that error changed as well. "Invalid Transaction: Payment" is far too vague, and doesn't tell you that there is no liquidity pool for this asset.

[ryanleecode]

@TarikGul @IkerAlus How does this look.

Maybe it would be nice to have it expandable, so it wouldnt take up so much space, but it would still need to have a preview text, which goes back to the question of how to render it if you don't know the shape.

[TarikGul]

@ryanleecode Looks really solid, nice job!

For the expendable could we do something like:

<tr>
    <td className='label'>{t('asset')}</td>
    <td className='data'>
        <details>
            <summary>{'{...}'}</summary>
            <pre>{someValueForJSOn}</pre>
        </details>
    </td>
</tr>

[ryanleecode]

@ryanleecode Looks really solid, nice job!

For the expendable could we do something like:

<tr>
    <td className='label'>{t('asset')}</td>
    <td className='data'>
        <details>
            <summary>{'{...}'}</summary>
            <pre>{someValueForJSOn}</pre>
        </details>
    </td>
</tr>

the thing i dont like about this is you need that extra click to see the actual asset id where as before it was assetId: 41 so you could easily tell what the asset was and sign it.

even worse is if the xcm is really big u might even need to scroll down. (not the case in the picture below, but imagine if it was a X3 xcm).

[TarikGul]

the thing i don't like about this is you need that extra click to see the actual asset id where as before it was assetId: 41 so you could easily tell what the asset was and sign it.

I think there is two key points here that i see:

• The user should have visibility into the exact information they inputted to build the payload. A displayed u32 is much different than a MultiLocation in terms of what the user inputs.
  • The GeneralIndex is only part of the information that characterizes the asset. For example: The value passed into the PalletInstance is just as important as the GeneralIndex, as well as the parents. So being able to verify each field of the Multilocation would be super useful for the user.

[ryanleecode]

the thing i don't like about this is you need that extra click to see the actual asset id where as before it was assetId: 41 so you could easily tell what the asset was and sign it.

I think there is two key points here that i see:

• The user should have visibility into the exact information they inputted to build the payload. A displayed u32 is much different than a MultiLocation in terms of what the user inputs.

  • The GeneralIndex is only part of the information that characterizes the asset. For example: The value passed into the PalletInstance is just as important as the GeneralIndex, as well as the parents. So being able to verify each field of the Multilocation would be super useful for the user.

Ideally there should be a tool out there to parse the MultiLocation into its written format. I haven't found one such tool so I've opened an issue in xcm-tools to request this: https://github.com/paraspell/xcm-tools/issues/188

[IkerAlus]

Ideally there should be a tool out there to parse the MultiLocation into its written format. I haven't found one such tool so I've opened an issue in xcm-tools to request this: paraspell/xcm-tools#188

I agree it will be nicer to show the location in the written format. Assuming we go this way, will the interface show the whole location at first sight? or will the user still need to do the extra click?

[ryanleecode]

Ideally there should be a tool out there to parse the MultiLocation into its written format. I haven't found one such tool so I've opened an issue in xcm-tools to request this: paraspell/xcm-tools#188

I agree it will be nicer to show the location in the written format. Assuming we go this way, will the interface show the whole location at first sight? or will the user still need to do the extra click?

Shows the written format and can be expanded to show the whole thing.

[TarikGul]

So I looked into the xcm-tools and I am weary of adding another dependency, example comment https://github.com/polkadot-js/apps/pull/10078#issuecomment-1851375493.

One main thing is xcm-tools uses polkadot-js as direct dependency meaning the package will always need to be in sync with our updates which brings further maintenance, and potential complications. @Tbaut Any thoughts about the design.

My thought is we just keep the raw JSON as the displayed output with the dropdown, and avoid any additional maintenance of deps in the future.

[ryanleecode]

So I looked into the xcm-tools and I am weary of adding another dependency, example comment https://github.com/polkadot-js/apps/pull/10078#issuecomment-1851375493.

One main thing is xcm-tools uses polkadot-js as direct dependency meaning the package will always need to be in sync with our updates which brings further maintenance, and potential complications. @Tbaut Any thoughts about the design.

My thought is we just keep the raw JSON as the displayed output with the dropdown, and avoid any additional maintenance of deps in the future.

Its possible they make it a standalone package (they already have a monorepo structure in place). in theory the package should require no dependencies on its own, since the XCM format is known.

[ryanleecode]

@TarikGul Its in their pipeline for early April.

https://github.com/paraspell/xcm-tools/issues/188#issuecomment-2025319103

Shall we wait or get this merged with (...) for now and open another issue to followup

[TarikGul]

@TarikGul Its in their pipeline for early April.

paraspell/xcm-tools#188 (comment)

Shall we wait or get this merged with (...) for now and open another issue to followup

Happy to get this in, and we can create a tracking issue to follow this up.

[Tbaut]

nice, I think the JSON preview is good enough.

[polkadot-js-bot]

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.