-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Integrate substrate connect with Apps #5644
Integrate substrate connect with Apps #5644
Conversation
…under. Add translations; Alter Api.tsx in order to choose between light clients and/or url addresses
…ct already has the information included and will return error if no supported network is found
…troduce light client type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to have this. some small comments all around.
As for the test failure here - https://github.com/polkadot-js/apps/pull/5644/checks?check_run_id=2875618334#step:3:518 Adapt this - https://github.com/polkadot-js/apps/blob/master/jest.config.cjs#L28 |
I am not sure why the CIs started to break (I don't understand the error but something seems to be wrong with .yarn). |
It is a yarn.lock merge issue. Just copy the yarn.lock from master and replace it here. There should be no updates needed to it at all. |
…kod/apps into nik-integrate-w-substrate-connect
Played through. It seems to do what it says on the tin, which is great. There is a small issue - when the user swaps to a light client (and he has nothing), the screen just goes white and stays that way. We need some of fallback for that. Apart from that, seems great. EDIT: We also need somewhere to prompt people how and what to add. (But can do that as a follow-up, not even sure how/where and what something like that would look like) |
What do you mean by "He has nothing" ?
There are for now the substrate-connect landing-page and api docs - Im not sure if that is what we could use though |
@jacogr This is a little tricky. Steps: Please try to test with Westend as I am not sure why Polkadot/Kusama not always respond (I'll cc @tomaka here) You should see something like the lines below running in your console:
|
To give a little overview: Since smoldot is a light client, it doesn't store the state of the chain. Instead, it queries it from other nodes. In order to circumvent that, we only report substrate-connect as "connected" when it has synced to the head of the chain. Once at the head of the chain, all storage queries normally succeed. This works well for Westend. For Kusama and Polkadot, unfortunately, there is only one "WebSocket secure" node to connect to at the moment for each chain (cc https://github.com/paritytech/devops/issues/948). When the node is full, smoldot can't sync to the head of the chain at all, meaning that no "connected" event is reported to PolkadotJS. It should work when using the substrate-connect browser extension, as this unlocks more nodes to connect to. |
The point is that I'm still in white-screen mode on a local server - So, the issue is that people are going to click this, get exactly the same and have no way of recovering. The only reason I could recover here to swap to westend is because there is a bug where a "remove params" and refreshing doesn't actually load the light client again. (So it connected to the default chain and then allowed recovered after the params were removed) |
The white screen has to do with the Due to the fact that this exists in a Component ( I will be working on this to resolve it asap - if there is something that can be used (e.g. useApi) and I am not aware of please for your feedback @jacogr |
catch-22 - the |
…urned from substrate-connect works
@jacogr After further investigation and re-tries it was fairly easy to fix the "await" part but due to the fact that api was undefined a huge amount of fixes should take place across all Apps codebase in order to make sure that there will be no breaking. Instead we proceed with a workaround on the substrate-connect side that would resolve the issue and make the Apps code clearer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems sane after resolving the conflicts. Will just do a final play-through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Seems to do exactly what it says on the tin, this is an excellent addition.
Thank you.
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary:
This PR is introducing the (experimental) light client (smoldot) into the Polkadot JS apps throush
substrate-connect
.Visible Changes:
A new "server" is introduced under 'Live Relays & Parachains', 'Polkadot' and 'Kusama' and under 'Test Relays & Parachains', 'Westend' as can be seen below:
Description:
Substrate connect nodes can be connected either through in-browser light client or through extension.
Development process:
In order to make this as less intrusive as possible the following steps took place:
rpc
for existing nodes (e.g.?rpc=wss%3A%2F%2Frpc.polkadot.io#/explorer
) - a new url param is introduced calledsc
(from substrate-connect) that has the name of the node and the characteristic-substrate-connect
following (e.g.?sc=kusama-substrate-connect#/explorer
). This way it is easier a differentiation between the two, without altering existing implementation.Api.tsx
) a choice is made on whether substrate-connect's Detector will be used or existing implementation).packages/apps-config/src/endpoints/util.ts
) for endpoints that receives the url and the type ('json-rpc
orsubstrate-connect
- defaults to first if none given) and returns the url type, reducing this way duplication of code and typos).