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.

Sign up for GitHub

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

Jump to bottom

Smoldot should check the API version of the runtime specs before doing a call #949

Open
tomaka opened this issue Jun 17, 2021 · 4 comments
Open

Smoldot should check the API version of the runtime specs before doing a call #949

tomaka opened this issue Jun 17, 2021 · 4 comments

Comments

@tomaka
Copy link
Contributor

tomaka commented Jun 17, 2021

Right now smoldot just assumes that runtime functions exist and match a certain API. Unfortunately, the runtime function APIs can change while keeping the same name. The authoritative thing here is the apis field of the runtime spec.
@tomaka tomaka mentioned this issue Aug 9, 2021
Closed
@tomaka
Copy link
Contributor Author

tomaka commented Feb 1, 2022

cc #1975

@tomaka
Copy link
Contributor Author

tomaka commented Feb 1, 2022

I believe a good way to do this is to build the CoreVersion in HostVmPrototype::new and then hold it in the HostVmPrototype.
Then the runtime_host and read_only_runtime_host should require an API name and version.

Also relates to #1107

@tomaka
Copy link
Contributor Author

tomaka commented Jul 25, 2022

According to paritytech/substrate#8688, the apis field is actually more or less deprecated.
It seems that the design of Substrate does not consist in checking apis.

This issue could be closed as "wontfix", but I'm leaving it open because we need a substitute mechanism and I don't know which yet.

@tomaka tomaka mentioned this issue Oct 8, 2022
Merged
mergify bot pushed a commit that referenced this issue Oct 10, 2022
@tomaka
Refactor chain information building process (#2845)
6072ccc 
Close #1681

Right now the code of `warp_sync` and `chain_spec` both build a
`ChainInformation` struct by making runtime calls. However the code of
these two modules is completely different.
Because these two modules were built very early on in smoldot's history,
they are getting help from the sub-modules of the `chain_information`
module, that are also very old.

This PR modernizes all of this: it removes all the sub-modules under
`chain_information` and merges them into one that does everything.

This new module is now used by `warp_sync` and `chain_spec`, and
`warp_sync` and `chain_spec` are now more simple.

I'm also planning to use this new module in order to refactor the SQLite
database. (#1752)

This PR slightly changes the behavior of the warp syncing: we now
download the runtime, then build it, and then only download the call
proofs to obtain the chain information. Right now all the downloads are
done in parallel.
This change is in principle more correct. At the moment we blindly call
functions that we expect to exist. By downloading and building the
runtime first, we can check the list of supported functions and then
only call them. (cc #949) This is really not a problem at the moment
because none of the functions that we use has ever been deprecated, but
it could have become one in the future.
@tomaka tomaka mentioned this issue Nov 10, 2022
Merged
@tomaka
Copy link
Contributor Author

tomaka commented Nov 10, 2022 •
edited
Loading

According to paritytech/substrate#8688, the apis field is actually more or less deprecated.
It seems that the design of Substrate does not consist in checking apis.

This turned out to be completely wrong. The field isn't deprecated at all.

mergify bot added a commit that referenced this issue Nov 11, 2022
@tomaka @mergify
Check runtime API versions in the JSON-RPC layer (#2995)
a1c9198 
Fix #2974
cc #949

This PR modifies `runtime_call` to support an "API version constraint".
This constraint is then verified.
A `runtime_call_no_api_check` function has also been added as an escape
hatch for the `state_call` JSON-RPC function.

It also updates the `payment_info` module to account for
paritytech/substrate#12633

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@tomaka tomaka mentioned this issue Feb 9, 2023
Open
@tomaka tomaka mentioned this issue May 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@tomaka