Skip to content
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

Parachain light clients #8050

Merged
merged 6 commits into from
Aug 26, 2022
Merged

Parachain light clients #8050

merged 6 commits into from
Aug 26, 2022

Conversation

arrudagates
Copy link
Contributor

This PR defines the URL structure for parachain light clients as light://substrate-connect/relay/parachain, adds the functionality in, and also includes 2 parachain light clients!

Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

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

Thank you for this. I like the idea, I just cannot get over the feeling that

(a) the const paraOrRelay should be an in-line switch (no need for the extra function there)
(b) it will become very unmanageable, very soon

On the second point I don't have a better idea right now. Possibly we could rather do something like (untested, messy) -

import * as lightSpecs from './light-client-specs';

...
return paraArg && lightSpecs[paraArg]
  ? new ScProvider(JSON.stringify(lightSpecs[paraArg]), relay)
  : relay

So basically it only needs to be added as a spec and it will work (instead of adding it as a spec and then also adding it to a switch...)

@arrudagates
Copy link
Contributor Author

Thank you for this. I like the idea, I just cannot get over the feeling that

(a) the const paraOrRelay should be an in-line switch (no need for the extra function there)
(b) it will become very unmanageable, very soon

On the second point I don't have a better idea right now. Possibly we could rather do something like (untested, messy) -

import * as lightSpecs from './light-client-specs';

...
return paraArg && lightSpecs[paraArg]
  ? new ScProvider(JSON.stringify(lightSpecs[paraArg]), relay)
  : relay

So basically it only needs to be added as a spec and it will work (instead of adding it as a spec and then also adding it to a switch...)

About the first point, I'm a rust dev trying to write JavaScript 😅
I'll look into both today!

@jacogr
Copy link
Member

jacogr commented Aug 26, 2022

lol, no issues. TS is not quite Rust. Sometimes it will indeed drive you up the wall. It is what it is.

@arrudagates
Copy link
Contributor Author

Your suggestion actually worked perfectly, I just refactored the way the specs are imported and exported to more closely resemble the way things are done in apps-config.

@jacogr
Copy link
Member

jacogr commented Aug 26, 2022

Thank you - one question before I pull it down, have you connected to these chains to see if it does work? (The logic is correct exactly as the connect team suggests, the reason for asking is that I have had reports where parachains do not work via light)

@arrudagates
Copy link
Contributor Author

arrudagates commented Aug 26, 2022

Thank you - one question before I pull it down, have you connected to these chains to see if it does work? (The logic is correct exactly as the connect team suggests, the reason for asking is that I have had reports where parachains do not work via light)

Well, there were some issues, but they were all rectified before this PR was opened. This PR fixed the last issue that I had: paritytech/smoldot#2686 (This is an issue that, among other things, prevents chains with longer prefix accounts to submit extrinsics)
And to give a more direct answer to your question, yeah, they work!
image
image

Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

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

Thank you!

@jacogr jacogr merged commit 0266224 into polkadot-js:master Aug 26, 2022
@arrudagates
Copy link
Contributor Author

You better get ready for the masses to follow and start listing their parachain specs too hahaha!

@polkadot-js-bot
Copy link

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.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Aug 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants