Use augmented and generated types #1703

0xverin · 2023-05-16T15:39:52Z

resolves #1673
resolves #1667
resolves #1679 (@polkadot/api @polkadot/types @polkadot/kering)

@polkadot/typegen still cannot be upgraded,this issue comment has been mentioned.

I used the latest typegen to generate types for litentry-parachain-metadata. Because when I use litentry-sidechain-metadata, I found that the types are incorrect.
All the generated types have been successfully imported, and in the code, we can see what type it is, as shown in the screenshot below:

It is no longer like this:

Kailai-Wang · 2023-05-16T19:28:22Z

There's some misunderstanding, I think.

We mainly need the primitives types for sidechain, e.g. LitentryIdentity, LitentryValidationData etc. We hope these types are generated instead of being manually defined. That's why we need the sidechain metadata.

The parachain metadata only adds types/extrinsics for the parachain, which is nice-to-have, but not the main goal of the issue.
I'm unsure if we can merge both metadata into one, please check it.

Both parachain and sidechain have a pallet IdentityManagement, if that's a problem, we should check if metadata supports alias. If not, we need to change the pallet name in the worst case.

0xverin · 2023-05-17T04:50:12Z

After checking, it is not possible to directly generate LitentryIdentity and LitentryValidationData using any kind of metadata.

The argument-types used for these two types are also being manually defined.
I have examined the code of idhub, and the result is the same.So, if the sidechain doesn't have the types we need, I believe there may be no need for a merge.
I couldn't find any information on merging metadata in either the official documentation or the GitHub repository. I believe it may not be possible to merge metadata.

grumpygreenguy · 2023-05-17T12:48:06Z

I have examined the code of idhub, and the result is the same.

You mean that the generated types in the idhub code are the same we have? 🤔

0xverin · 2023-05-17T13:09:46Z

As of now, it appears to be the case.

I have examined the code of idhub, and the result is the same.

You mean that the generated types in the idhub code are the same we have? 🤔

Not entirely,they made changes to the content of identity/definitions, so augment-types.ts will have some differences. What I actually meant was that the generation rules are the same. However, that is not enough for us. I am currently thinking about how to include the types generated from sidechain-meta as well.

0xverin · 2023-05-24T13:55:09Z

Updated:

There was a mistake earlier:when querying the storage in the sidechain, the data passed to createType should come from the sidechain's construction, not the parachain's. The reason it appears to work is that the data structure of LitentryIdentity is correct and matches the expected format.
Although we have generated the LitentryPrimitivesIdentity type from sidechain, the data composed of this structure is still the same as before. I don't think I have made a mistake(if I have,plz correct me,thanks!). Here is an example of how Phala uses XcmVersionedXcm type. Additionally, if the returned structure is not like the one constructed by buildIdentityHelper,, it is unlikely to succeed.So,I didn't change LitentryIdentity type.
This pr, with the addition of tsconfig.json, type checking has been enabled, which means that when I pull other PRs, I encounter type check errors locally. It was my mistake, and in fact, the type checking should have been added to ts-test from the beginning instead of modifying the types now. Type checking is something that should be done from the start.

tee-worker/ts-tests/common/utils/identity-helper.ts

tee-worker/ts-tests/identity.test.ts

grumpygreenguy · 2023-05-25T12:13:13Z

tee-worker/ts-tests/package.json

+		"gen-type:parachain": "yarn gen-type-parachain:defs && yarn gen-type-parachain:meta && yarn format",
+		"gen-type-parachain:defs": "ts-node --skip-project node_modules/.bin/polkadot-types-from-defs  --package . --input ./parachain-interfaces --endpoint ./litentry-parachain-metadata.json",
+		"gen-type-parachain:meta": "ts-node --skip-project node_modules/.bin/polkadot-types-from-chain --package . --output ./parachain-interfaces --endpoint ./litentry-parachain-metadata.json",
+		"gen-type:sidechain": "yarn gen-type-sidechain:defs && yarn gen-type-sidechain:meta && yarn format",
+		"gen-type-sidechain:defs": "ts-node --skip-project node_modules/.bin/polkadot-types-from-defs  --package . --input ./sidechain-interfaces --endpoint ./litentry-sidechain-metadata.json",
+		"gen-type-sidechain:meta": "ts-node --skip-project node_modules/.bin/polkadot-types-from-chain --package . --output ./sidechain-interfaces --endpoint ./litentry-sidechain-metadata.json",


Do we have these documented somewhere? If so then that probably needs updated as well 🤔

grumpygreenguy

LGTM; left minor comments/questions

Kailai-Wang

I'd expect things like LitentryIdentity and LitentryValidationData would be deleted from type-definitions.ts and types in metadata are used instead. Isn't it the case?

Kailai-Wang · 2023-05-25T15:00:01Z

tee-worker/ts-tests/tsconfig.json

+		"paths": {
+			"@polkadot/api/augment": ["./parachain-interfaces/augment-api.ts"],
+			"@polkadot/types/augment": ["./parachain-interfaces/augment-types.ts"],
+			"@polkadot/types/lookup": ["./sidechain-interfaces/types-lookup.ts"]


These three paths are a mixture of parachain and sidechain interfaces, is this intended?

They are different. The first two refer to the detection of API types (for example, when using api.tx, we need the parachain type). The last one refers to the configuration when we need to import a specific type (for example, importing LitentryPrimitivesIdentity).

What if the types that we need are in the parachain types-lookup?

I have contemplated merging them together: "@polkadot/types/lookup":[parachain..., sidechain...], but it is not supported. There are related issues mentioning this, so I think ultimately Rust needs to merge identity types into parachain.

tee-worker/ts-tests/common/utils/identity-helper.ts

tee-worker/ts-tests/common/type-definitions.ts

0xverin · 2023-05-31T07:28:53Z

Updated:

Removed custom types related to identity and replaced them with generated types.
Modified parts of the code that involved assertions to use interfaces from the autogenerated types.

tee-worker/ts-tests/batch.test.ts

0xverin · 2023-06-01T15:43:52Z

Major updates:

Refactored all the assertions.
I have made an effort to use the LitentryPrimitivesIdentity interface for data comparison between events and idGraph. I believe this approach is reasonable(usage).

Once the CI passes, it is ready for review.

grumpygreenguy · 2023-06-02T11:51:47Z

tee-worker/ts-tests/identity.test.ts

+const substrateExtensionIdentity = {
    Substrate: {
        address: '0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48', //Bob
        network: 'Litentry',
    },
-} as any;
+} as unknown as LitentryPrimitivesIdentity;


I get why we need the as when interfacing with the (mis-)augmented API, but why the assertion here?

If I get it correctly this is the input we want to pass to createType('LitentryPrimitivesIdentity', <_>), but from what we saw earlier that type is actually different from how LitentryPrimitivesIdentity is declared, no? For instance, this input doesn't have the type or isSubstrate props.

This might still be a job for the ExtractEnumVariant we discussed earlier 🤔

This might still be a job for the ExtractEnumVariant we discussed earlier

Your understanding is correct. If we want perfection, we do need to do it that way(ExtractEnumVariant). However, you will find that in our ts-test, we mostly use interfaces for LitentryPrimitivesIdentity. I'm thinking that when assembling the parameters, we can define the LitentryPrimitivesIdentity type ,but need pass it to createType with Substrate: { address: '0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48', network: 'Litentry', }. Do you think this is significant? Would it be enough for IDHub to know the LitentryPrimitivesIdentity which interfaces it has?

If they want to define LitentryPrimitivesIdentity, would it be enough for them to assemble their own ExtractEnumVariant?

If we want perfection, we do need to do it that way(ExtractEnumVariant).

I don't think we want perfection, but we should at least strive to have accurate definitions for the input and output datatypes in the API (even if we cannot get them to be inferred correctly in each usage).

However, you will find that in our ts-test, we mostly use interfaces for LitentryPrimitivesIdentity.

I don't follow what you mean by this 🤔

but need pass it to createType with Substrate: { address: '0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48', network: 'Litentry', }.

My point is that AFAICT this value ({ Substrate: { ... } }) that we're passing to createType is not a LitentryPrimitivesIdentity, but rather some other type that at the moment doesn't have a name. We can derive that type from LitentryPrimitivesIdentity with something like ExtractEnumVariant, but they're not the same type and I think it only makes sense to make that distinction explicit.

We have decided to address this matter in another PR/issue.

0xverin · 2023-06-05T06:58:32Z

Now there is a final decision. Do we want to assemble the ExtractEnumVariant type to fully reconstruct the LitentryPrimitivesIdentity type? Personally, I believe it may be more meaningful for IDHub to understand how to use the interfaces of LitentryPrimitivesIdentity. They may need more interfaces to perform certain state checks (I inferred this from looking at their code, as they also use as unknown). They can refer to our usage of assert to utilize the LitentryPrimitivesIdentity interface. @Kailai-Wang @grumpygreenguy :P

Since this PR has been open for a while, I would like to make the final confirmation in the last stage. We can create a separate PR for the "looks-up" issue because that seems to be a TypeScript problem.

Kailai-Wang · 2023-06-05T18:01:49Z

I still see many conflicting files for this PR.
Can we resolve these ASAP and get it merged (if it's ready)? This PR has been there for 3 weeks :(

0xverin added 8 commits May 16, 2023 20:20

types generated with parachain-metadata

02c0d2e

change gen-type

62078af

tsconfig.json

6baea67

add env

01c6829

upgrade typegen && re-generate

cb4ebbc

fix package conflict

88946b5

fix types

750d4fa

remove comments

f844315

0xverin added 2 commits May 17, 2023 10:41

fix ci

d8d5a0e

upgradte polkadot<api types>

939e871

0xverin requested review from Kailai-Wang, grumpygreenguy and higherordertech May 17, 2023 04:50

0xverin mentioned this pull request May 17, 2023

Typegen Usage which worked with 9.14.2 doesn't work with 10.1.2 polkadot-js/api#5540

Closed

4 tasks

upgrade @polkador/kering

317b8ab

0xverin mentioned this pull request May 17, 2023

Can polkadot.js/api merge two metadata into one metadata and generate types for use? polkadot-js/api#5646

Closed

0xverin added 3 commits May 18, 2023 18:34

add sidechain-types

2b583d6

add @polkadot/types/lookup

1e00b98

Merge branch 'dev' into 1673-use-generated-types

de37126

grumpygreenguy approved these changes May 23, 2023

View reviewed changes

grumpygreenguy self-requested a review May 23, 2023 12:32

0xverin added 3 commits May 24, 2023 20:35

fix createType

9324355

Merge branch 'dev' into 1673-use-generated-types

5ed93e0

remove toHex()

4355e7b

fix ci & fix type

5e7662f

grumpygreenguy reviewed May 25, 2023

View reviewed changes

tee-worker/ts-tests/common/utils/identity-helper.ts Outdated Show resolved Hide resolved

grumpygreenguy reviewed May 25, 2023

View reviewed changes

tee-worker/ts-tests/common/utils/identity-helper.ts Outdated Show resolved Hide resolved

grumpygreenguy reviewed May 25, 2023

View reviewed changes

tee-worker/ts-tests/identity.test.ts Outdated Show resolved Hide resolved

grumpygreenguy reviewed May 25, 2023

View reviewed changes

Kailai-Wang reviewed May 25, 2023

View reviewed changes

0xverin added 4 commits May 31, 2023 14:52

refactor identity&validation types

18c8e50

Merge branch 'dev' into 1673-use-generated-types

3dc50f0

fix checkIdgraph

e78a07a

remove WorkerRpcReturnValue

ad8e9cc

grumpygreenguy reviewed May 31, 2023

View reviewed changes

tee-worker/ts-tests/batch.test.ts Outdated Show resolved Hide resolved

grumpygreenguy reviewed May 31, 2023

View reviewed changes

tee-worker/ts-tests/batch.test.ts Outdated Show resolved Hide resolved

0xverin added 4 commits May 31, 2023 16:37

fix ci & format

8e3ea50

modified as type

b082d30

modified Assertion type

472d4ca

refactor assert

f85448a

fix vc ci

bda99a7

0xverin force-pushed the 1673-use-generated-types branch from 7ffe8e5 to bda99a7 Compare June 1, 2023 16:04

add storage query sleep

514f003

0xverin force-pushed the 1673-use-generated-types branch from 575f7b2 to 514f003 Compare June 1, 2023 17:39

grumpygreenguy reviewed Jun 2, 2023

View reviewed changes

Merge branch 'dev' into 1673-use-generated-types

6631fc0

0xverin merged commit 5a3b76e into dev Jun 7, 2023

0xverin mentioned this pull request Jun 7, 2023

ts-tests: use the derived types of LitentryPrimitivesIdentity to construct an identity. #1754

Closed

grumpygreenguy mentioned this pull request Jun 7, 2023

fix DI examples #1755

Merged

0xverin deleted the 1673-use-generated-types branch August 9, 2023 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use augmented and generated types #1703

Use augmented and generated types #1703

0xverin commented May 16, 2023 •

edited

Loading

Kailai-Wang commented May 16, 2023

0xverin commented May 17, 2023

grumpygreenguy commented May 17, 2023

0xverin commented May 17, 2023

0xverin commented May 24, 2023 •

edited

Loading

grumpygreenguy May 25, 2023

grumpygreenguy left a comment

Kailai-Wang left a comment

Kailai-Wang May 25, 2023

0xverin May 31, 2023

Kailai-Wang May 31, 2023

0xverin Jun 1, 2023

0xverin commented May 31, 2023

0xverin commented Jun 1, 2023 •

edited

Loading

grumpygreenguy Jun 2, 2023

0xverin Jun 5, 2023

grumpygreenguy Jun 5, 2023

0xverin Jun 7, 2023

0xverin commented Jun 5, 2023

Kailai-Wang commented Jun 5, 2023

Use augmented and generated types #1703

Use augmented and generated types #1703

Conversation

0xverin commented May 16, 2023 • edited Loading

Kailai-Wang commented May 16, 2023

0xverin commented May 17, 2023

grumpygreenguy commented May 17, 2023

0xverin commented May 17, 2023

0xverin commented May 24, 2023 • edited Loading

Choose a reason for hiding this comment

grumpygreenguy left a comment

Choose a reason for hiding this comment

Kailai-Wang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xverin commented May 31, 2023

0xverin commented Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xverin commented Jun 5, 2023

Kailai-Wang commented Jun 5, 2023

0xverin commented May 16, 2023 •

edited

Loading

0xverin commented May 24, 2023 •

edited

Loading

0xverin commented Jun 1, 2023 •

edited

Loading