Skip to content
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.

Sign up for GitHub

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

Jump to bottom

AssetId always converted to Multilocation or errors with numbers other than 0 #5846

Open
3 of 6 tasks
Robiquet opened this issue Apr 11, 2024 · 12 comments
Open
3 of 6 tasks

AssetId always converted to Multilocation or errors with numbers other than 0 #5846

Robiquet opened this issue Apr 11, 2024 · 12 comments
Labels
Support Tracks issues or requests related to troubleshooting, answering questions, and user assistance.

Comments

@Robiquet
Copy link

Robiquet commented Apr 11, 2024 •
edited
Loading

Bug report

At Zeitgeist we use pallet-asset-tx-payment to pay fees in other currencies. We use it like this:

const keyring = new Keyring({ type: "sr25519" });
const alice = keyring.addFromUri("//Alice", { name: "Alice default" });
const signedTx = await tx.signAsync(alice, {assetId: 0});

However now that we are using the latest version assetId is being forced into a Multilocation or in other cases it just errors.

Here are some examples:

const signedTx = await tx.signAsync(alice, {assetId: 0});
console.log(signTx.assetId) // {"parents": "0","interior": "Here"}

const signedTx = await tx.signAsync(alice, {assetId: 1});
console.log(signTx.assetId) // "Error: Struct: failed on assetId: Option<MultiLocation>:: Struct: Cannot decode value 1 (typeof number), expected an input object, map or array"

Moving forward we would like to use arbitrary values like this but run into similar issues:

const signedTx = await tx.signAsync(alice, {assetId: { ForeignAsset: 0 }});
console.log(signTx.assetId) // {"parents": "0","interior": "Here"}

const signedTx = await tx.signAsync(alice, {assetId: { ForeignAsset: 1 }});
console.log(signTx.assetId) // {"parents": "0","interior": "Here"}

Seems related to the changes made in this pr: #5752

  • Please tell us about your environment:

  • Version: 10.12.6

  • Environment:

    • Node.js
    • Browser
    • Other (limited support for other environments)

  • Language:

    • JavaScript
    • TypeScript (include tsc --version)
    • Other
@TarikGul
Copy link
Member

If I am not mistaken you can add any typesBundle or typesSpec fields to override certain types when instantiating the ApiPromise. And you would set TAssetConversion to Option<AssetId> as seen here in the PR you referenced.

Moving forward we would like to use arbitrary values like this but run into similar issues:

I don't see the api supporting an input like { ForeignAsset: 1 }. IMO the asset passed in should always be a valid and explicit MultiLocation. Short cuts especially for something as specific as a multiLocation can cause unwanted behavior or bugs.

@sea212
Copy link

sea212 commented Apr 11, 2024

Is there a way to use anything else than MultiLocation as AssetId?

One pallet used to allow transaction fee payment with assets is a standard Substrate frame pallet pallet-asset-tx-payment. In the transaction validation function, it uses an AssetId that can be specified by the developer. It seems wrong to me to force the asset id type within the sdk when it is configurable in the pallet that processes the asset id on the blockchain side.

@TarikGul
Copy link
Member

Is there a way to use anything else than MultiLocation as AssetId?

Yes, you should be able to do this by following what I wrote in the above:

If I am not mistaken you can add any typesBundle or typesSpec fields to override certain types when instantiating the ApiPromise. And you would set TAssetConversion to Option as seen here in the PR you referenced.

@Robiquet
Copy link
Author

Thanks @TarikGul we managed to get it working with this change and signing with a keyring pair, see transaction here. However when we try to sign with an extension we get the following error:

image

I'm guessing this means that we need to add the overrides to this repo in a similar fashion to the asset hubs, what do you think?

@Robiquet
Copy link
Author

Hi @TarikGul would you be able to take a look at this and let us know the best course of action?

@TarikGul
Copy link
Member

I'm guessing this means that we need to add the overrides to this repo in a similar fashion to the asset hubs, what do you think?

I am not completely sure to be honest, what version of the extension are you using?

@Robiquet
Copy link
Author

I'm using version 0.44.1. I think this is the issue because it looks very similar to this. It looks like the reason for it was that these changes to override the types had not yet been deployed. So it seems like we will need to add similar overrides for Zeitgeist

@gagi13
Copy link

gagi13 commented Jun 4, 2024

Hi guys, happy this thread exists 😁

We are developing the UI for ACP and ran into the same issue. So when we tried to sign the transaction with a non-DOT token as gas fee token via extension, it broke and we got the same error as above. Any updates on resolving this or any workaround you found? @Robiquet

FYI @fmol2y

@Robiquet
Copy link
Author

Robiquet commented Jun 10, 2024 •
edited
Loading

@gagi13 I'm not too sure about that as the correct overrides should already be there for the asset hubs. I would try different wallets and versions of polkadot js. It seems like there hasn't been a release published for polkadot js extension for a while now.

@TarikGul
Copy link
Member

We are very close to releasing a new version of the extension.

Also it might be worth checking to see if this works against the most recent version of the extension in github, instructions on how to run it in the README

@TarikGul
Copy link
Member

New versions of the extension have both been released for a while now: @Robiquet is this still an issue?

@Robiquet
Copy link
Author

New versions of the extension have both been released for a while now: @Robiquet is this still an issue?

Yeah the new release of the extension has made things worse for us, now neither approach works. Seems like overrides within polkadot js api are needed, the same sort of changes that were done to support the asset hubs

@TarikGul TarikGul added the Support Tracks issues or requests related to troubleshooting, answering questions, and user assistance. label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Support Tracks issues or requests related to troubleshooting, answering questions, and user assistance.
Projects
Polkadot-js general project board
Status: Support
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Robiquet @sea212 @TarikGul @gagi13