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

Refactor the host <-> runtime interface machinery #3689

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
20fab05
Refactor the host <-> runtime interface machinery
koute Mar 14, 2024
e854ff3
Rename the prdoc file to the correct PR number
koute Mar 14, 2024
b1346cc
Remove redundant lifetimes
koute Mar 14, 2024
5180738
Fix crate name in prdocs
koute Mar 14, 2024
2745774
Remove more redundant lifetimes
koute Mar 14, 2024
d13c67f
Also adjust FRAME benchmarking runtime interface
koute Mar 14, 2024
e0fb03b
Update `polkadot-runtime-metrics` crate
koute Mar 14, 2024
3b87116
Remove `sp-std` dependency from `sp-wasm-interface`
koute Mar 14, 2024
1aab744
Add missing imports to `sp-runtime-interface`
koute Mar 14, 2024
64c37e5
Make `sp_wasm_interface::Result` always return a `String` as an error
koute Mar 14, 2024
d71e093
Make `sp-runtime-interface` always `#[no_std]`
koute Mar 14, 2024
4cd6caa
Merge remote-tracking branch 'origin/master' into master_runtime_inte…
koute Mar 18, 2024
b8aebe1
Switch `runtime-interface` macros to use `substrate_runtime`
koute Mar 18, 2024
feeda1a
Fix `check-features-variants.sh` scripts
koute Mar 18, 2024
11dfa9f
More `feature = "std"` cleanups; fix `full-crypto` feature compilation
koute Mar 18, 2024
8a8dcb0
Update `secp256k1` and `secp256k1-sys` to fix WASM compilation
koute Mar 19, 2024
1163801
Fix `sp-io` compilation under non-runtime `no_std`
koute Mar 19, 2024
af46473
Build frame examples with `--cfg substrate_runtime` on the CI
koute Mar 19, 2024
0a254f5
Propagate `sp-core/std` -> `futures/std` cargo feature
koute Mar 19, 2024
7ff56da
Merge remote-tracking branch 'origin/master' into master_runtime_inte…
koute Mar 19, 2024
2763e81
Fix `pallet_ui` tests
koute Mar 20, 2024
a3dee1f
Merge 'origin/master'; update `crypto_bytes.rs`
koute Mar 20, 2024
022c32c
Disable default features for `sp-externalities` dep in `sp-core`
koute Mar 20, 2024
9a446a8
More `feature = "std"` cleanups to fix `full_crypto` compilation
koute Mar 20, 2024
29c8ddf
Merge remote-tracking branch 'origin/master' into master_runtime_inte…
koute Apr 5, 2024
1e53458
Remove unnecessary `RIType` and `IntoFFIValue` impls
koute Apr 5, 2024
c240e1e
Use associated types in return types
koute Apr 5, 2024
6ac3d06
Make `Primitive` safe and private
koute Apr 5, 2024
ba683e7
Cleanups
koute Apr 5, 2024
1cb22ca
Rename `PassByCodec` to `PassFatPointerAndDecode`
koute Apr 5, 2024
6e606ad
Rename `PassSliceRefByCodec` -> `PassFatPointerAndDecodeSlice`
koute Apr 5, 2024
6d90a1d
Update doc comment
koute Apr 5, 2024
0001db0
Update the docs about the deprecated marshaling strategies
koute Apr 5, 2024
182a6e8
Remove redundant links in the docs
koute Apr 5, 2024
589690d
Merge remote-tracking branch 'origin/master' into master_runtime_inte…
koute Apr 8, 2024
c794600
Update prdoc
koute Apr 8, 2024
a6af3b2
Merge remote-tracking branch 'origin/master' into master_runtime_inte…
koute Apr 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion polkadot/runtime/metrics/src/with_runtime_metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,5 +143,5 @@ impl Histogram {

/// Returns current time in ns
pub fn get_current_time() -> u128 {
frame_benchmarking::benchmarking::current_time()
frame_benchmarking::current_time()
}
83 changes: 83 additions & 0 deletions prdoc/pr_3689.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# yaml-language-server: $schema=https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Refactor the host <-> runtime interface machinery (the `#[runtime_interface]` macro) and the way host functions are defined

doc:
- audience: Runtime Dev
- audience: Node Dev
description: |
This PR refactors the way the host functions are defined.

Previously the way a given type was marshalled through the host <-> runtime boundary was
hardcoded for every type by the virtue of it implementing the relevant conversion traits.

This had two major consequences:
* It was not obvious how a given type is going to be passed just by looking at its type alone,
masking potentially expensive marshalling strategies. (For example, returning `Option<u32>`
was done through the SCALE codec and involved extra memory allocations!)
* It was not possible to use multiple marshalling strategies for a single type, making some
of the future improvements we'd like to do (e.g. move the runtime memory allocator into the runtime)
very hard to do.

So this PR disentangles this mess and makes the marshalling strategies explicit. This makes it
much more clear how a given type in a given host function is marshalled, and also makes it possible
to use different marshalling strategies for the same type in different host functions.

Before this PR you'd define a host function like this:

```rust
#[runtime_interface]
trait MyInterface {
fn say_hello_world(name: &str) {
println!("Hello {name}!");
}
}
```

and after this PR you'll define it like this:

```rust
#[runtime_interface]
trait MyInterface {
fn say_hello_world(name: PassFatPointerAndRead<&str>) {
println!("Hello {name}!", name);
}
}
```

In this case the strategy for passing the `&str` is now explicitly specified (`PassFatPointerAndRead`).
Note that the *actual* API generated by this macro and the way arguments are accessed is completely unchanged!
The `#[runtime_interface]` machinery automatically "strips" away the marshalling strategy wrappers,
so neither the body of the `say_hello_world` function here nor its callers need to be changed.
crates:
- name: sp-runtime-interface
bump: major
note: Rework of the `#[runtime_interface]` macro and associated types/trait.
- name: sp-runtime-interface-proc-macro
bump: major
note: Rework of the `#[runtime_interface]` macro.
- name: sp-wasm-interface
bump: major
note: The `Pointer` type now implements `Copy` and `Clone` unconditionally. The `Result` now always returns a `String`.
- name: sp-core
bump: major
note: Some types don't implement the traits related to the old `#[runtime_interface]` anymore. A few extra conversion impls.
- name: sp-io
bump: major
note: Requires the new `#[runtime_interface]` macro and associated machinery. Some types don't implement the traits related to the old `#[runtime_interface]` anymore.
- name: sp-statement-store
bump: major
note: Requires the new `#[runtime_interface]` macro and associated machinery. Some types don't implement the traits related to the old `#[runtime_interface]` anymore.
- name: sp-crypto-ec-utils
bump: minor
note: Requires the new `#[runtime_interface]` macro and associated machinery.
- name: frame-benchmarking
bump: major
note: Requires the new `#[runtime_interface]` macro and associated machinery. `Benchmarking::current_time` host call was changed.
- name: frame-support-procedural
bump: minor
note: Needs new `frame-benchmarking` due to the change to `current_time`.
- name: polkadot-runtime-metrics
bump: minor
note: Needs new `frame-benchmarking` due to the change to `current_time`.
4 changes: 2 additions & 2 deletions substrate/client/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,11 +741,11 @@ impl<D: NativeExecutionDispatch> sp_core::traits::ReadRuntimeVersion for NativeE
#[cfg(test)]
mod tests {
use super::*;
use sp_runtime_interface::runtime_interface;
use sp_runtime_interface::{pass_by::PassFatPointerAndRead, runtime_interface};

#[runtime_interface]
trait MyInterface {
fn say_hello_world(data: &str) {
fn say_hello_world(data: PassFatPointerAndRead<&str>) {
println!("Hello world from: {}", data);
}
}
Expand Down
34 changes: 25 additions & 9 deletions substrate/frame/benchmarking/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ use scale_info::TypeInfo;
use serde::{Deserialize, Serialize};
use sp_io::hashing::blake2_256;
use sp_runtime::{traits::TrailingZeroInput, DispatchError};
use sp_runtime_interface::pass_by::{
AllocateAndReturnByCodec, PassByCodec, PassFatPointerAndRead, PassPointerAndWrite,
};
use sp_std::{prelude::Box, vec::Vec};
use sp_storage::TrackedStorageKey;

Expand Down Expand Up @@ -242,18 +245,29 @@ sp_api::decl_runtime_apis! {
}
}

/// Get the number of nanoseconds passed since the UNIX epoch
///
/// WARNING! This is a non-deterministic call. Do not use this within
/// consensus critical logic.
pub fn current_time() -> u128 {
let mut out = [0; 16];
self::benchmarking::current_time(&mut out);
u128::from_le_bytes(out)
}

/// Interface that provides functions for benchmarking the runtime.
#[sp_runtime_interface::runtime_interface]
pub trait Benchmarking {
/// Get the number of nanoseconds passed since the UNIX epoch
///
/// WARNING! This is a non-deterministic call. Do not use this within
/// consensus critical logic.
fn current_time() -> u128 {
std::time::SystemTime::now()
fn current_time(out: PassPointerAndWrite<&mut [u8; 16], 16>) {
*out = std::time::SystemTime::now()
.duration_since(std::time::SystemTime::UNIX_EPOCH)
.expect("Unix time doesn't go backwards; qed")
.as_nanos()
.to_le_bytes();
}

/// Reset the trie database to the genesis state.
Expand All @@ -267,7 +281,7 @@ pub trait Benchmarking {
}

/// Get the read/write count.
fn read_write_count(&self) -> (u32, u32, u32, u32) {
fn read_write_count(&self) -> AllocateAndReturnByCodec<(u32, u32, u32, u32)> {
self.read_write_count()
}

Expand All @@ -277,17 +291,17 @@ pub trait Benchmarking {
}

/// Get the DB whitelist.
fn get_whitelist(&self) -> Vec<TrackedStorageKey> {
fn get_whitelist(&self) -> AllocateAndReturnByCodec<Vec<TrackedStorageKey>> {
self.get_whitelist()
}

/// Set the DB whitelist.
fn set_whitelist(&mut self, new: Vec<TrackedStorageKey>) {
fn set_whitelist(&mut self, new: PassByCodec<Vec<TrackedStorageKey>>) {
self.set_whitelist(new)
}

// Add a new item to the DB whitelist.
fn add_to_whitelist(&mut self, add: TrackedStorageKey) {
fn add_to_whitelist(&mut self, add: PassByCodec<TrackedStorageKey>) {
let mut whitelist = self.get_whitelist();
match whitelist.iter_mut().find(|x| x.key == add.key) {
// If we already have this key in the whitelist, update to be the most constrained
Expand All @@ -306,18 +320,20 @@ pub trait Benchmarking {
}

// Remove an item from the DB whitelist.
fn remove_from_whitelist(&mut self, remove: Vec<u8>) {
fn remove_from_whitelist(&mut self, remove: PassFatPointerAndRead<Vec<u8>>) {
let mut whitelist = self.get_whitelist();
whitelist.retain(|x| x.key != remove);
self.set_whitelist(whitelist);
}

fn get_read_and_written_keys(&self) -> Vec<(Vec<u8>, u32, u32, bool)> {
fn get_read_and_written_keys(
&self,
) -> AllocateAndReturnByCodec<Vec<(Vec<u8>, u32, u32, bool)>> {
self.get_read_and_written_keys()
}

/// Get current estimated proof size.
fn proof_size(&self) -> Option<u32> {
fn proof_size(&self) -> AllocateAndReturnByCodec<Option<u32>> {
self.proof_size()
}
}
Expand Down
8 changes: 4 additions & 4 deletions substrate/frame/benchmarking/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,11 +1109,11 @@ macro_rules! impl_benchmark {
);

let start_pov = $crate::benchmarking::proof_size();
let start_extrinsic = $crate::benchmarking::current_time();
let start_extrinsic = $crate::current_time();

closure_to_benchmark()?;

let finish_extrinsic = $crate::benchmarking::current_time();
let finish_extrinsic = $crate::current_time();
let end_pov = $crate::benchmarking::proof_size();

// Calculate the diff caused by the benchmark.
Expand All @@ -1140,9 +1140,9 @@ macro_rules! impl_benchmark {
);

// Time the storage root recalculation.
let start_storage_root = $crate::benchmarking::current_time();
let start_storage_root = $crate::current_time();
$crate::__private::storage_root($crate::__private::StateVersion::V1);
let finish_storage_root = $crate::benchmarking::current_time();
let finish_storage_root = $crate::current_time();
let elapsed_storage_root = finish_storage_root - start_storage_root;

let skip_meta = [ $( stringify!($name_skip_meta).as_ref() ),* ];
Expand Down
8 changes: 4 additions & 4 deletions substrate/frame/support/procedural/src/benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,11 +630,11 @@ pub fn benchmarks(
);

let start_pov = #krate::benchmarking::proof_size();
let start_extrinsic = #krate::benchmarking::current_time();
let start_extrinsic = #krate::current_time();

closure_to_benchmark()?;

let finish_extrinsic = #krate::benchmarking::current_time();
let finish_extrinsic = #krate::current_time();
let end_pov = #krate::benchmarking::proof_size();

// Calculate the diff caused by the benchmark.
Expand All @@ -657,9 +657,9 @@ pub fn benchmarks(
);

// Time the storage root recalculation.
let start_storage_root = #krate::benchmarking::current_time();
let start_storage_root = #krate::current_time();
#krate::__private::storage_root(#krate::__private::StateVersion::V1);
let finish_storage_root = #krate::benchmarking::current_time();
let finish_storage_root = #krate::current_time();
let elapsed_storage_root = finish_storage_root - start_storage_root;

let skip_meta = [ #(#skip_meta_benchmark_names_str),* ];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#!/usr/bin/env -S bash -eux

export RUSTFLAGS="-Cdebug-assertions=y -Dwarnings"
cargo check --release

export RUSTFLAGS="$RUSTFLAGS --cfg substrate_runtime"
T=wasm32-unknown-unknown
cargo check --release
cargo check --release --target=$T --no-default-features
cargo check --release --target=$T --no-default-features --features="full_crypto"
cargo check --release --target=$T --no-default-features --features="serde"
Expand Down
10 changes: 5 additions & 5 deletions substrate/primitives/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ ss58-registry = { version = "1.34.0", default-features = false }
sp-std = { path = "../std", default-features = false }
sp-debug-derive = { path = "../debug-derive", default-features = false }
sp-storage = { path = "../storage", default-features = false }
sp-externalities = { path = "../externalities", optional = true }
futures = { version = "0.3.21", optional = true }
dyn-clonable = { version = "0.9.0", optional = true }
thiserror = { optional = true, workspace = true }
tracing = { version = "0.1.29", optional = true }
bitflags = "1.3"
Expand All @@ -65,6 +62,11 @@ w3f-bls = { version = "0.1.3", default-features = false, optional = true }
# bandersnatch crypto
bandersnatch_vrfs = { git = "https://github.com/w3f/ring-vrf", rev = "e9782f9", default-features = false, features = ["substrate-curves"], optional = true }

[target.'cfg(not(substrate_runtime))'.dependencies]
sp-externalities = { path = "../externalities" }
futures = { version = "0.3.21", default-features = false, features = ["alloc"] }
dyn-clone = "1.0.17"

[dev-dependencies]
criterion = "0.4.0"
serde_json = { workspace = true, default-features = true }
Expand All @@ -89,10 +91,8 @@ std = [
"bounded-collections/std",
"bs58/std",
"codec/std",
"dyn-clonable",
"ed25519-zebra/std",
"full_crypto",
"futures",
"futures/thread-pool",
"hash-db/std",
"hash256-std-hasher/std",
Expand Down
2 changes: 1 addition & 1 deletion substrate/primitives/core/check-features-variants.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env -S bash -eux

export RUSTFLAGS="-Cdebug-assertions=y -Dwarnings"
export RUSTFLAGS="-Cdebug-assertions=y -Dwarnings --cfg substrate_runtime"
T=wasm32-unknown-unknown

cargo check --target=$T --release --no-default-features --features="bls-experimental"
Expand Down
24 changes: 8 additions & 16 deletions substrate/primitives/core/src/bandersnatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ use bandersnatch_vrfs::{CanonicalSerialize, SecretKey};
use codec::{Decode, Encode, EncodeLike, MaxEncodedLen};
use scale_info::TypeInfo;

use sp_runtime_interface::pass_by::PassByInner;
use sp_std::{vec, vec::Vec};

/// Identifier used to match public keys against bandersnatch-vrf keys.
Expand All @@ -54,21 +53,16 @@ const PREOUT_SERIALIZED_SIZE: usize = 33;

/// Bandersnatch public key.
#[derive(
Clone,
Copy,
PartialEq,
Eq,
PartialOrd,
Ord,
Encode,
Decode,
PassByInner,
MaxEncodedLen,
TypeInfo,
Hash,
Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Encode, Decode, MaxEncodedLen, TypeInfo, Hash,
)]
pub struct Public(pub [u8; PUBLIC_SERIALIZED_SIZE]);

impl From<[u8; PUBLIC_SERIALIZED_SIZE]> for Public {
fn from(raw: [u8; PUBLIC_SERIALIZED_SIZE]) -> Self {
Public(raw)
}
}

impl UncheckedFrom<[u8; PUBLIC_SERIALIZED_SIZE]> for Public {
fn unchecked_from(raw: [u8; PUBLIC_SERIALIZED_SIZE]) -> Self {
Public(raw)
Expand Down Expand Up @@ -150,9 +144,7 @@ impl<'de> Deserialize<'de> for Public {
///
/// The signature is created via the [`VrfSecret::vrf_sign`] using [`SIGNING_CTX`] as transcript
/// `label`.
#[derive(
Clone, Copy, PartialEq, Eq, Encode, Decode, PassByInner, MaxEncodedLen, TypeInfo, Hash,
)]
#[derive(Clone, Copy, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, Hash)]
pub struct Signature([u8; SIGNATURE_SERIALIZED_SIZE]);

impl UncheckedFrom<[u8; SIGNATURE_SERIALIZED_SIZE]> for Signature {
Expand Down
Loading
Loading