parsing assetId from a json payload fails since v13.0.1

[0xKheops]

• I'm submitting a ...

• Bug report
• Feature request
• Support request
• Other

• What is the current behavior and expected behavior?

From a user perspective, on https://kheopswap.xyz/#/polkadot/transfer , when transfering a AH asset, the transaction is rejected if user selected a fee asset (ex: USDT) to pay for fees. This occurs with both Talisman and polkadot.js wallets.

Note: we upgraded @polkadot/* dependencies in Talisman few weeks back.
With a build from before that upgrade, signing those transactions was working fine.

I identified that since @polkadot/api v13, the assetId value cannot be parsed from a signer json payload anymore. This was working fine up to version v12.4.2

• What is the motivation for changing the behavior?

• Please tell us about your environment:

Kheopswap is developed with PAPI, which is used to generate the JSON payload used in the example below.
Tagging @josepot just in case.

I don't know any other dapp that allows selecting the fee asset so I've only tested it there.

I've prepared this repo to demonstrate the issue : https://github.com/0xKheops/issue-pjs-ah-signing
Based on the version of @polkadot/api the assetId can be parsed or not.
v12.4.2 or below: OK
v13.0.1 or above: assetId can't be parsed

• Version:

• Environment:

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

• Language:

  • JavaScript
  • TypeScript (include tsc --version)
  • Other

[josepot]

I'd like to take this opportunity to point out -once again- that the SignerPayloadJSON interface has been causing significant challenges and is hindering the ecosystem's progress. It would be beneficial to replace this interface with a non-leaky, library-agnostic alternative, as suggested in this GitHub comment.

One of the deliverables of our latest OpenGov proposal was to work on "Extension support". To make progress in this area, it's crucial that we improve this common interface and decouple it from the Polkadot JS internals. I initially thought that Parity would prioritize this work, but it appears it's not on their roadmap. Therefore, the Polkadot-API team has decided to take the initiative on this task and will adjust our priorities accordingly.

In the meantime, I suggest that extensions consider using the @polkadot-api/tx-utils library. It includes a fromPjsToTxData helper that could help address this issue.

[TarikGul]

@0xKheops Ahh thanks for revealing the issue above with the ExtrinsicPayload construction. I'll get that fixed - should be somewhat straight forward, that being said there really should have been testing on that front (which is a mishap on my end).

@josepot All the help I can get is highly appreciated. With the end of the year closing in my bandwidth really is just spread super thin. Is it internally known that this interface is an area of friction and needs to be improved/changed/reworked - very much so yes. At the end of the day, the PAPI team has been pretty instrumental in helping PJS and I when fighting this interface, and I am extremely grateful. I'll be here to add any support necessary moving forward. I just don't have the individual capacity to tackle the whole thing on my own.

[TarikGul]

@0xKheops Ahh I think I see the real issue here. Moving forward the assetId as hex is expected to be its exact type which in this case is a Option<MultiLocation>, therefore the hex should actually be 0x010002043205011f instead of 0x0002043205011f.

Took me a second to notice and remember that. That being said the PR above also adds tests in the future to ensure that the behavior is not changed.

[TarikGul]

For example the release notes from 13.0.1:

Breaking Changes:

Change assetId type in SignerPayloadJSON to HexString (https://github.com/polkadot-js/api/pull/5967)
Keep assetId hex as an Option for toPayload (https://github.com/polkadot-js/api/pull/5968)

NOTE: This is part of the current change to generalize the SignerPayloadJSON in order to simplify its integration across other tools. The return value of assetId field with toPayload will always be its true type now, which in this case is Option<TAssetConversion> where TAssetConversion is equal to MultiLocation | AssetId.

I think the release notes there could have been a bit more specific as well to note when passing in a payload where the assetId is a hex, it should explicitly represent its on chain type. In this case Option<MultiLocation>

[TarikGul]

Closing as I believe this is expected, and the correct behavior. The above PR should also add some safeguards to ensure this is not tampered with in the future.

[josepot]

For example the release notes from 13.0.1:

Breaking Changes:

Change assetId type in SignerPayloadJSON to HexString (https://github.com/polkadot-js/api/pull/5967)
Keep assetId hex as an Option for toPayload (https://github.com/polkadot-js/api/pull/5968)

NOTE: This is part of the current change to generalize the SignerPayloadJSON in order to simplify its integration across other tools. The return value of assetId field with toPayload will always be its true type now, which in this case is Option<TAssetConversion> where TAssetConversion is equal to MultiLocation | AssetId.

I think the release notes there could have been a bit more specific as well to note when passing in a payload where the assetId is a hex, it should explicitly represent its on chain type. In this case Option<MultiLocation>

And how in the world is the consumer supposed to know whether the extension that is communicating with expects the binary data with or without the Option bytes?

[0xKheops]

Ok thank you, will rollback to v12 until a consensus is found :)

[TarikGul]

And how in the world is the consumer supposed to know whether the extension that is communicating with expects the binary data with or without the Option bytes?

The idea here is to reflect exactly what the metadata states, and what is expected in the final extrinsic. This was requested from Carlos to set the SignerPayloadJSON assetId field to be a hex value of the actual type reflected on chain.

[polkadot-js-bot]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.