XCM crates squating breaks types

[wirednkod]

• I'm submitting a ...

  • Bug report
  • Feature request
  • Support request
  • Other

• What is the current behavior and expected behavior?

Due to some XCM crates squating there is a renaming of crates. As a result the latest metadata are having a Staking prefix which (apparently) broke teleporting in test faucet

Normally this "check" on client side should not take place and types (e.g. XcmVersionedMultiLocation should not appear like StakingXcmVersionedMultiLocation)

• What is the motivation for changing the behavior?

Current breaking of test faucet (rococo) and possible breakings in other apps

• Please tell us about your environment:

• Version: 10.10.1

• Environment:

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

• Language:

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

[bkchr]

@jacogr appending the paths in front of the types is really not a great idea. I think you probably tried to make them unique in this way. However, this is no guarantee that there doesn't exist a different type with the same name + prefix.

[bkchr]

I think users probably only require the type when they want to instantiate a Call. Instead of them trying to instantiate the type on their own, polkadot-js should give them the type by calling something like type_for("my_call", index). Then they could for example get the correct type for xcm to instantiate it.

[jacogr]

There are no clashes - when a clash happens, it exposes the type by the Lookup id only.

(These metadata type ids change with each and every runtime upgrade, so you end up in a much worse position using the ids plus some prefix. The type ids/indexes are far from stable, it is the things that changes the most each and every upgrade.)

Types are generated for each metadata, so when (like in this case), there is a rename, the types will be different. So the breakage here is certainly expected, the type names are generated based on what is in the metadata.

TL;DR The code behaves as expected. Nothing to do here, the names changed, the exposed types changed. These are generated automatically.

[bkchr]

I mean I get that the code works as expected. However, I think that people should not write their code in this way. They should not try to search for a type by name and instead get the type they are interested in by index from a call. This should be much more stable. I mean I'm also open to other suggestions. However for decoding we shouldn't have this problem as polkadot-js knows the types and can decode it properly. The only problem should be around encoding for transactions that we have seen now.

[jacogr]

You are never supposed to work with the type names directly.

So you should never do -

const something = api.createType('SomeType', { foo: 'bar' });
...
await api.tx.somewhere.something(something);
...

In all cases you should just let the API create the types, aka -

await api.tx.somewhere.something({ foo: 'bar' });

So if you don't use API do do the internal conversion and instead use a name-based createType yourself, you will be into a world of hurt. Never do that. Let the API create since it always knows what is exposed.

So we can go round and round, it stays a wontfix.

[bkchr]

Ahh okay thank you!

[wirednkod]

Thank you @jacogr and @bkchr . Will close this here

[mutantcornholio]

@jacogr is that articulated somewhere? Maybe it should be mentioned here, at least?
https://polkadot.js.org/docs/api/start/types.create

Until the issue happened, we had no idea that this is a bad practice, and I can easily find tons of cases where people do it

[TarikGul]

Just to add: I do think there is merit behind using api.createType. In a static environment for testing it can be incredibly useful. Especially when you want to mock storage queries from the chain for testing purposes.

Here is an example below:

// This was taken directly from polkadot-js
export function createApiWithAugmentations(metadataHex: `0x${string}`): ApiPromise {
	const registry = new TypeRegistry();
	const metadata = new Metadata(registry, metadataHex);

	registry.setMetadata(metadata);

	const api = new ApiPromise({
		provider: new WsProvider('ws://', false),
		registry,
	});

	api.injectMetadata(metadata, true, registry);

	return api;
}

const mockApi = createApiWithAugmentations(<some_metadata>);

const getSafeXcmVersion = () =>
	Promise.resolve().then(() => {
		return mockApi.registry.createType('Option<u32>', 2);
	});
	
const adjustedMockApi = {
   registry: mockApi.registry
   query: {
       polkadotXcm: {
           safeXcmVersion: getSafeXcmVersion
       }
   }
}

I also think it can be really useful when constructing the Call type or ExtrinsicPayload types for decoding. But generally speaking I do agree that in the case of building a tx.something it makes sense to just pass the params as is.

Edit: fix typo.

[IkerAlus]

@jacogr is that articulated somewhere? Maybe it should be mentioned here, at least? https://polkadot.js.org/docs/api/start/types.create

Until the issue happened, we had no idea that this is a bad practice, and I can easily find tons of cases where people do it

@TarikGul just created a PR for said doc page: polkadot-js/docs#455

[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.