runtime-interface: Implement `register_only` functions #10640

bkchr · 2022-01-11T23:01:17Z

The runtime interface supports versioning of functions. Currently, if you add a new function it will
be used by the runtime automatically. This results in requiring all nodes of a network to upgrade
before the runtime is upgraded, otherwise they will fail to instantiate the new runtime because of
missing host functions. This pr introduces register_only functions. This can be used when a new
runtime interface function should be introduced, but the actual usage can be deferred. This means
that nodes will have the host function for this, but the runtime will still use the old version of
the function when being compiled for wasm. However, when a runtime is enacted that uses the new host
function, the "old nodes" will already have the host function and will continue to work.

The runtime interface supports versioning of functions. Currently, if you add a new function it will be used by the runtime automatically. This results in requiring all nodes of a network to upgrade before the runtime is upgraded, otherwise they will fail to instantiate the new runtime because of missing host functions. This pr introduces `register_only` functions. This can be used when a new runtime interface function should be introduced, but the actual usage can be deferred. This means that nodes will have the host function for this, but the runtime will still use the old version of the function when being compiled for wasm. However, when a runtime is enacted that uses the new host function, the "old nodes" will already have the host function and will continue to work.

cheme

LGTM, I am not too sure how things will behave in practice. For instance if/when changing

substrate/primitives/io/src/lib.rs

Line 397 in fb24fda

#[version(2)]

to register_only, runtime code will need to be patched back to _v1 prototype (without the version). But it also reduces the surface of code to patch, so looks good.

primitives/runtime-interface/proc-macro/src/utils.rs

cheme · 2022-01-12T10:11:02Z

primitives/runtime-interface/proc-macro/src/utils.rs

+/// While this struct is only for parsing the inner parts inside the `()`.
+struct VersionAttribute {
+ version: u32,
+ register_only: Option<attributes::register_only>,


Could be just bool? (also fine this way)

Yeah, but I use this struct here directly for parsing. So, I decided to do it this way.

cheme · 2022-01-12T10:14:19Z

primitives/runtime-interface/src/lib.rs

@@ -153,6 +153,22 @@ pub use sp_std;
 /// [17].to_vec()
 /// }
 ///
+/// /// Call function, different version and only being registered.
+/// ///
+/// /// This `register_only` version is only being registered, aka exposed to the runtime,


Suggested change

/// /// This `register_only` version is only being registered, aka exposed to the runtime,

/// /// `register_only` indicates version is only being registered, aka exposed to the runtime,

cheme · 2022-01-12T10:28:10Z

primitives/runtime-interface/src/lib.rs

+/// /// nodes have the host functions available, because otherwise they fail at instantiation
+/// /// of the runtime. With `register_only` the function will not be used when compiling the
+/// /// runtime, but it will already be there for a future version of the runtime that will
+/// /// switch to using these host function.


Just to be sure I catch the intent and implication.
So the switch will be done by removing register_only and will require nodes updates (client hard fork).
So the PR makes it possible to stage some pending new function, but will still need a big update later.
The update may require some parachain to patch this flag until they do the client/node, can be a bit of work if some tests does need switching when removing the flag.
I feel like I am back in the situation where I would put this behind a feature (not a great idea I know).
Thinking twice, it feels good this way, the patch to retain register would be very small and the client update would in most case need to happen soon (to get latest runtime changes).

The whole point of this is the following. Let's imagine we want to add new host functions, like with your trie pr. Currently we only we add these functions and directly start using them from the runtime. This way, the moment a runtime using these new host functions is enacted, the node needs to have this host functions ready or it starts to fail importing new blocks. With parachains and the relay chain node currently being compiled into the parachain node it even means that parachain teams need to bring out a new node release on time as well just to have the host functions ready for the relay chain. This is really shitty.

With register_only, we will be able to prepare the node side. Aka we will already be able to ship the new host functions now, but the runtime associated to the release or even the one after that don't use the new host functions. It will then be enabled at some later point when we are aware that most of the network has upgraded.

This can not be done by features, because using a feature would completely remove the host function and again we would not have it in the node. The feature itself could then be used to disable the runtime functionality and maybe for also making removing the register_only when it is time.

primitives/runtime-interface/src/lib.rs

cheme · 2022-01-12T10:33:38Z

primitives/runtime-interface/test-wasm/src/lib.rs

@@ -123,6 +123,15 @@ pub trait TestApi {
 data == 42


weird indent (may be github).

Yeah that is github, it is fucking it up for me as well.

Co-authored-by: cheme <emericchevalier.pro@gmail.com>

bkchr · 2022-01-15T10:46:08Z

bot merge

paritytech-processbot · 2022-01-15T10:46:18Z

Bot will approve on the behalf of @bkchr, since they are a team lead, in an attempt to reach the minimum approval count

) * runtime-interface: Implement `register_only` functions The runtime interface supports versioning of functions. Currently, if you add a new function it will be used by the runtime automatically. This results in requiring all nodes of a network to upgrade before the runtime is upgraded, otherwise they will fail to instantiate the new runtime because of missing host functions. This pr introduces `register_only` functions. This can be used when a new runtime interface function should be introduced, but the actual usage can be deferred. This means that nodes will have the host function for this, but the runtime will still use the old version of the function when being compiled for wasm. However, when a runtime is enacted that uses the new host function, the "old nodes" will already have the host function and will continue to work. * Update primitives/runtime-interface/src/lib.rs Co-authored-by: cheme <emericchevalier.pro@gmail.com> * Update primitives/runtime-interface/proc-macro/src/utils.rs Co-authored-by: cheme <emericchevalier.pro@gmail.com> * FMT Co-authored-by: cheme <emericchevalier.pro@gmail.com>

bkchr requested review from andresilva and cheme January 11, 2022 23:01

cheme approved these changes Jan 12, 2022

View reviewed changes

bkchr and others added 3 commits January 15, 2022 09:20

Update primitives/runtime-interface/src/lib.rs

be52124

Co-authored-by: cheme <emericchevalier.pro@gmail.com>

Update primitives/runtime-interface/proc-macro/src/utils.rs

97e98b6

Co-authored-by: cheme <emericchevalier.pro@gmail.com>

FMT

bbde555

paritytech-processbot bot approved these changes Jan 15, 2022

View reviewed changes

paritytech-processbot bot merged commit 3168e10 into master Jan 15, 2022

paritytech-processbot bot deleted the bkchr-runtime-interface-register-only branch January 15, 2022 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime-interface: Implement `register_only` functions #10640

runtime-interface: Implement `register_only` functions #10640

bkchr commented Jan 11, 2022

cheme left a comment

cheme Jan 12, 2022

bkchr Jan 15, 2022

cheme Jan 12, 2022

cheme Jan 12, 2022

bkchr Jan 15, 2022

cheme Jan 12, 2022

bkchr Jan 15, 2022

bkchr commented Jan 15, 2022

paritytech-processbot bot commented Jan 15, 2022

	/// /// This `register_only` version is only being registered, aka exposed to the runtime,
	/// /// `register_only` indicates version is only being registered, aka exposed to the runtime,

runtime-interface: Implement register_only functions #10640

runtime-interface: Implement register_only functions #10640

Conversation

bkchr commented Jan 11, 2022

cheme left a comment

Choose a reason for hiding this comment

cheme Jan 12, 2022

Choose a reason for hiding this comment

bkchr Jan 15, 2022

Choose a reason for hiding this comment

cheme Jan 12, 2022

Choose a reason for hiding this comment

cheme Jan 12, 2022

Choose a reason for hiding this comment

bkchr Jan 15, 2022

Choose a reason for hiding this comment

cheme Jan 12, 2022

Choose a reason for hiding this comment

bkchr Jan 15, 2022

Choose a reason for hiding this comment

bkchr commented Jan 15, 2022

paritytech-processbot bot commented Jan 15, 2022

runtime-interface: Implement `register_only` functions #10640

runtime-interface: Implement `register_only` functions #10640