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

Use substrate image, that is kept up-to-date #778

Closed
masapr opened this issue Jun 13, 2024 · 4 comments · Fixed by #802
Closed

Use substrate image, that is kept up-to-date #778

masapr opened this issue Jun 13, 2024 · 4 comments · Fixed by #802
Assignees
Labels
F2-bug Something isn't working Stay-up-to-date

Comments

@masapr
Copy link
Collaborator

masapr commented Jun 13, 2024

With the current image we use: paritypr/substrate:latest we have the issue, that it is not updated. We should change to parity/polkadot (if possible). This seems to be the official docker image and is currently updated frequently. Also note the current bugreport: #761

We had changed from parity/substrate:latest to paritypr/substrate:latest here: #720

@Niederb
Copy link
Contributor

Niederb commented Jul 3, 2024

I did a quick test with switching to the parity/polkadot image, but a wide range of tests fail:
https://github.com/scs/substrate-api-client/actions/runs/9776877250
It's not clear to me why this is and unfortunately I have not seen any documentation regarding the docker images and their differences so far.

@haerdib
Copy link
Contributor

haerdib commented Sep 2, 2024

Switching to a polkadot node requires switching the api-client runtime as well. Otherwise, the api-client will use different types than the node. See the comment in the examples:

https://github.com/scs/substrate-api-client/blob/master/examples/async/examples/get_storage.rs#L28-L31

I did a quick research on the missing pallets of the polkdot node (according to the docker README, it's the westend runtime)
The following pallet is missing in the westend runtime, which we currently run examples against:

So we would need to remove this example or at least mark it as untested.

Other than that, Alice still seems to be sudo and the unstable rpc api should be the same as well. So I think the rest should work as is, with very little changes. Haven't tried it yet, though - there might be some hidden caveats. This would also solve
#383

As lots of our user are having trouble with the switching of the runtime (therefore the issue #383), I think it's worth the switch, even though we would lose a (tested) example.

@Niederb @masapr what do you think?

@masapr
Copy link
Collaborator Author

masapr commented Sep 2, 2024

I think it's definitely worth a try to check how many tests will fail. So, I definitely vote for making a PR with the change and looking at the results from our tests. If we only lose the staking tests, I'd be fine with it.

@Niederb
Copy link
Contributor

Niederb commented Sep 3, 2024

I agree: If it is only the staking example that is affected then it is worth the change 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F2-bug Something isn't working Stay-up-to-date
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants