Skip to content
This repository was archived by the owner on Oct 10, 2022. It is now read-only.

Conversation

@weichweich
Copy link
Contributor

@weichweich weichweich commented Apr 28, 2022

Fixes https://github.com/KILTprotocol/ticket/issues/1953

It adds type augmentation for the runtime APIs (callable with api.call.didApi...), but it has on hold the generation of types for the DID RPC stuff, as decoding is currently failing when fetching data using a Typescript script.

Checklist

How to test

After #33 is merged into this, you can spin up a local instance of polkadotJS apps, where you replace the KILT type definition with a locally published version of this package (e.g. with yalc). You should now be able to see the pubilc credential RPCs among the possible RPC methods, and also the runtime calls for both the DID and the public credentials runtime APIs.

One screenshot for the RPC and one for the runtime calls attached.

Screenshot 2022-08-11 at 16 24 16

Screenshot 2022-08-11 at 16 25 30

How to integrate into the SDK

For usage with the SDK, the typegen package must be used, by first pulling in the metadata from an up-to-date chain that has these features (e.g. from develop), then updating packages/augment-api/src/interfaces/extraDefs/definitions.ts to use the rpc, types, and runtime definitions by pulling them in from this package, and the run build:types. The result should be an up-to-date typescript definition of all RPC and state_call stuff.

@ntn-x2
Copy link
Contributor

ntn-x2 commented Apr 28, 2022

We don't use this anymore in the SDK. The RPC info is populated directly from the runtime definition.

@weichweich
Copy link
Contributor Author

weichweich commented Apr 29, 2022

We don't use this anymore in the SDK. The RPC info is populated directly from the runtime definition.

The RPC information is not part of the runtime metadata and cannot be inferred. It has to be setup manually. (see blockoverflow)

Copy link
Contributor Author

@weichweich weichweich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to approve my on PR. But GitHub seems to be against that for what ever reason... 🙄

Copy link
Contributor

@ntn-x2 ntn-x2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@ntn-x2
Copy link
Contributor

ntn-x2 commented Jul 29, 2022

I have tested it with a local instance of the PolkadotJS Apps against both our latest develop and with Spiritnet and it seems to be working fine. To avoid any unnecessary risk with the official PolkadotJS Apps, I will leave this PR open and we will wait to merge it until we have started the release process for 1.8.0, at which point we will merge and release, and test the new types against Peregrine Staging.

@ntn-x2
Copy link
Contributor

ntn-x2 commented Sep 1, 2022

There is currently a limitation in how public keys are serialized in Substrate. They are ss58-encoded, and this means that there is need for some custom decoding logic on the client side. Commit 11eb4f8 fixes this by declaring the DID public verification keys as Text, which is the ss58-encoded representation of the public key. RPC consumers will have to call decodeAddress(public_key) to retrieve the byte representation of the key, if required.

Anyway, that was the last limitation for fully supporting the new RPC endpoints, hence we can now move on and test the new type definitions.

@ntn-x2 ntn-x2 requested a review from wischli September 1, 2022 13:04
@ntn-x2
Copy link
Contributor

ntn-x2 commented Sep 1, 2022

This PR has been re-scoped to support the 1.7.2 release, which adds support for the DID RPC endpoints, but does not include neither public credentials nor the new Ethereum account linking. Hence, those two features have been removed from this PR, and will be tracked in different ticekts. The goal of this PR is to make the type definitions ready for the next release.

Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I have one minor question though.

@ntn-x2 ntn-x2 self-assigned this Sep 2, 2022
@ntn-x2 ntn-x2 added the enhancement New feature or request label Sep 2, 2022
@ntn-x2 ntn-x2 merged commit 63d66e2 into master Sep 5, 2022
@ntn-x2 ntn-x2 deleted the aw-rpc-types branch September 5, 2022 08:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants