polkadot-js / api

Promise and RxJS APIs around Polkadot and Substrate based chains via RPC calls. It is dynamically generated based on what the Substrate runtime provides in terms of metadata.
Apache License 2.0
1.07k stars 349 forks source link

Wrong `OpaquePeerId` imported in TypeScript augmentation #5219

Open gdethier opened 2 years ago

gdethier commented 2 years ago

When generating custom typescript definitions for a custom Substrate node using the nodeAuthorization pallet in its runtime, there seems to be a kind of "clash" between nodeAuthorization's OpaquePeerId and imOnline's one. In order to prevent some double-encoding issue, we had to override (in definitions.ts file) OpaquePeerId so that it is defined as a Vec<u8> (and not a WrapperOpaque<Bytes> like in imOnline). This works in our case because we are not using the imOnline pallet.

However, even when overriding the type definition, imOnline's type keeps to be imported in the augment-*.ts files. The only workaround we have found so far is to fix the imports manually i.e. rewrite imports like

import type { OpaquePeerId } from '@polkadot/types/interfaces/imOnline';

into

import type { OpaquePeerId } from './default';

In order to reproduce:

You'll see that all OpaquePeerId are reset to point to imOnline's OpaquePeerId.

When a type is explicitly overridden in a definitions file, it should also be imported in the augment files.

jacogr commented 1 year ago

I believe the network layer ios actually problematic here, encoding things twice. Just looking at the use, it is meant to be Bytes (as per Rust), not Opaque<Bytes>, which makes the imOnline work, but is not really correct based on the Rust types.

So we probably need a specific override for the imOnline pallet to make the specific use there work. (And revert the type definition to match with Rust for the more general "available everywhere" case)