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.

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

PolkaJS unable to decode valid block #10432

Open
2 of 10 tasks
dandanlen opened this issue Apr 4, 2024 · 4 comments · May be fixed by polkadot-js/api#6089
Open
2 of 10 tasks

PolkaJS unable to decode valid block #10432

dandanlen opened this issue Apr 4, 2024 · 4 comments · May be fixed by polkadot-js/api#6089
Assignees
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. P4 - Needs Investigation Requires analysis to determine cause or feasibility. Not fully understood, needs research first.

Comments

@dandanlen
Copy link

  • I'm submitting a ...
  • Bug report
  • Feature request
  • Support request
  • Other
  • What is the current behavior and expected behavior?

Current behaviour: polkaJS is unable to decode this Chainflip block:

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fmainnet-archive.chainflip.io#/explorer/query/0xf7d516bd72ee1d76903e692157fc2a5260788faa6c9b6a369c3006b19413b6d2

Blocks before and after decode fine (2053085 is the block height of the failing block).

The Chainflip block explorer can decode the block:

https://scan.chainflip.io/blocks/2053085

The issue appears to be a type ChainAsset<T, I> which is an alias for a chain-specific enum of assets. (See 'asset' in Governance.propose_governance_extrinsic extrinsic in the Chainflip block explorer).

For example, there is an Ethereum instance of ChainAsset that resolves to an eth::Asset: enum Asset { Eth, Flip, Usdc, Usdt }. Another instance resolves to a btc::Asset: enum Asset { Btc }.

The problem seems to be that polkaJS is trying to resolve an encoded eth::Asset::Usdt to a btc::Asset. Since btc::Asset only has one variant, the decoding fails.

I'm not sure if the source of the issue is indeed polkaJS, or scale metadata, or a combination of the two.

I'm also not sure if this can be mitigated or avoided by declaring our substrate types a bit differently.

Any advice would be welcome.

  • What is the motivation for changing the behavior?

The encoding is correct, and is compatible with substrate / metadata. We use similar patterns elsewhere - this seems like something that polkaJs should be able to handle.

Some tools seem to be able to correctly decode this type using the metadata. For example, subxt codegen correctly disambiguates the types.

  • Please tell us about your environment:
  • Version:

  • Environment:

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

    • JavaScript
    • TypeScript (include tsc --version)
    • Other
@TarikGul TarikGul added Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. needs investigation labels Apr 8, 2024
@martin-chainflip
Copy link

Hi, this issue still occurs regularly for us, so I've spent some more time investigating it. I believe it boils down to the following discrepancy:
a) In the type registry, it is perfectly fine to define multiple types with the same name, they each get different type ids. For a given storage item, the type registry identifies the type of the storage item via this unique type id.
BUT
b) The rpc-core package identifies the type of a storage item via its name, (literally a string). Because of the way the internal lookup into the type registry works, if multiple types with the same name exist, the last one (with the largest type id) is used, which ultimately causes the above error.

I think that somewhere in this function https://github.com/polkadot-js/api/blob/a15d0dd0df6b0ba87d88eb7f637b9e1d6e0fb87a/packages/rpc-core/src/bundle.ts#L499
it should be possible to define the type used for registry.createTypeUnsafe() via the type id (integer) instead of the type name (string).

@TarikGul TarikGul self-assigned this Oct 28, 2024
@TarikGul TarikGul added P4 - Needs Investigation Requires analysis to determine cause or feasibility. Not fully understood, needs research first. and removed needs investigation labels Oct 30, 2024
@TarikGul TarikGul moved this from Issues (Bugs) to P4 - Needs Investigation in Polkadot-js general project board Oct 30, 2024
@TarikGul
Copy link
Member

TarikGul commented Nov 8, 2024

Hi, this issue still occurs regularly for us, so I've spent some more time investigating it. I believe it boils down to the following discrepancy: a) In the type registry, it is perfectly fine to define multiple types with the same name, they each get different type ids. For a given storage item, the type registry identifies the type of the storage item via this unique type id. BUT b) The rpc-core package identifies the type of a storage item via its name, (literally a string). Because of the way the internal lookup into the type registry works, if multiple types with the same name exist, the last one (with the largest type id) is used, which ultimately causes the above error.

I think that somewhere in this function https://github.com/polkadot-js/api/blob/a15d0dd0df6b0ba87d88eb7f637b9e1d6e0fb87a/packages/rpc-core/src/bundle.ts#L499 it should be possible to define the type used for registry.createTypeUnsafe() via the type id (integer) instead of the type name (string).

Thanks for doing the initial research, I am looking into this as we speak!

@dandanlen
Copy link
Author

@TarikGul here is some (possibly) relevant context - there was a similar issue with scale-typegen:

paritytech/scale-typegen#14

@martin-chainflip
Copy link

martin-chainflip commented Feb 4, 2025

For your information, it is possible to hack this to work, by not relying on the outputTypeName to be unique and instead constructing the lookup name.

in polkadot/rpc-core/bundle.js you could modify "_newType" like this

    _newType(registry, blockHash, key, input, isEmpty, entryIndex = -1) {
        const pallet = registry.metadata.pallets.find((e) => e.name.toString().toLowerCase() === key.section.toString().toLowerCase());
        const call = pallet?.storage.unwrap().items.find((e) => e.name.toString().toLowerCase() === key.method.toString().toLowerCase());
        const typeid = call?.type['value']['value'].toHuman();
        const type = typeid ? "Lookup" + typeid : "Raw";
        const meta = key.meta || EMPTY_META;
...

Essentially i lookup the typeid of the output value of the method in the registry and then use that instead of the name. It works for me, but obviously you could make this more elegant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. P4 - Needs Investigation Requires analysis to determine cause or feasibility. Not fully understood, needs research first.
Projects
Status: P4 - Needs Investigation
3 participants