Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Embed runtime version as a custom section #8688

Merged
17 commits merged into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
17 changes: 16 additions & 1 deletion Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ members = [
"primitives/trie",
"primitives/utils",
"primitives/version",
"primitives/version/proc-macro",
"primitives/wasm-interface",
"test-utils/client",
"test-utils/derive",
Expand Down
1 change: 1 addition & 0 deletions bin/node-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub mod opaque {

// To learn more about runtime versioning and what each of the following value means:
// https://substrate.dev/docs/en/knowledgebase/runtime/upgrades#runtime-versioning
#[sp_version::runtime_version]
pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("node-template"),
impl_name: create_runtime_str!("node-template"),
Expand Down
1 change: 1 addition & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ pub fn wasm_binary_unwrap() -> &'static [u8] {
}

/// Runtime version.
#[sp_version::runtime_version]
pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("node"),
impl_name: create_runtime_str!("substrate-node"),
Expand Down
1 change: 0 additions & 1 deletion client/executor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ sp-api = { version = "3.0.0", path = "../../primitives/api" }
sp-wasm-interface = { version = "3.0.0", path = "../../primitives/wasm-interface" }
sp-runtime-interface = { version = "3.0.0", path = "../../primitives/runtime-interface" }
sp-externalities = { version = "0.9.0", path = "../../primitives/externalities" }
sp-maybe-compressed-blob = { version = "3.0.0", path = "../../primitives/maybe-compressed-blob" }
sc-executor-common = { version = "0.9.0", path = "common" }
sc-executor-wasmi = { version = "0.9.0", path = "wasmi" }
sc-executor-wasmtime = { version = "0.9.0", path = "wasmtime", optional = true }
Expand Down
1 change: 1 addition & 0 deletions client/executor/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ wasmi = "0.6.2"
sp-core = { version = "3.0.0", path = "../../../primitives/core" }
sp-allocator = { version = "3.0.0", path = "../../../primitives/allocator" }
sp-wasm-interface = { version = "3.0.0", path = "../../../primitives/wasm-interface" }
sp-maybe-compressed-blob = { version = "3.0.0", path = "../../../primitives/maybe-compressed-blob" }
sp-serializer = { version = "3.0.0", path = "../../../primitives/serializer" }
thiserror = "1.0.21"

Expand Down
26 changes: 26 additions & 0 deletions client/executor/common/src/runtime_blob/runtime_blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ pub struct RuntimeBlob {
}

impl RuntimeBlob {
/// Create `RuntimeBlob` from the given wasm code. Will attempt to decompress the code before
/// deserializing it.
///
/// See [`sp_maybe_compressed_blob`] for details about decompression.
pub fn uncompress_if_needed(wasm_code: &[u8]) -> Result<Self, WasmError> {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe new should do this? IMHO the name indicates that this is done on an already existing reference, while actually we create an instance with this method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean because it's uncompress but returns Self? I don't think it is unheard of, even though that new_*, from_* or with_* are the most common. For example, there is TcpStream::connect which returns Self.

I also had an idea about putting it in new, but in the end I decided to go with uncompress to indicate at the call sites , mainly in sc-executor, that the code may be compressed at that point. If compression were hidden in new, it would be even harder to find out.

I do agree that it is a small thing and perhaps is not that useful, so I can tuck this logic under new if you wish so.

use sp_maybe_compressed_blob::CODE_BLOB_BOMB_LIMIT;
let wasm_code = sp_maybe_compressed_blob::decompress(wasm_code, CODE_BLOB_BOMB_LIMIT)
.map_err(|e| WasmError::Other(format!("Decompression error: {:?}", e)))?;
Self::new(&wasm_code)
}

/// Create `RuntimeBlob` from the given wasm code.
///
/// Returns `Err` if the wasm code cannot be deserialized.
Expand Down Expand Up @@ -85,9 +96,24 @@ impl RuntimeBlob {
})
}

/// Scans the wasm blob for the first section with the name that matches the given. Returns the
/// contents of the custom section if found or `None` otherwise.
pub fn custom_section_contents(&self, section_name: &str) -> Option<&[u8]> {
self.raw_module
.custom_sections()
.filter(|cs| cs.name() == section_name)
.next()
.map(|cs| cs.payload())
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.filter(|cs| cs.name() == section_name)
.next()
.map(|cs| cs.payload())
.filter_map(|cs| (cs.name() == section_name).then(|| cs.payload()))

Copy link
Contributor Author

@pepyakin pepyakin May 6, 2021

Choose a reason for hiding this comment

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

I am not entirely sure how would that work. Currently, this code takes an iterator of custom sections, leaves only sections that we are interested in, takes the first one which gives us an option and then we narrow the contents of this section to only its payload. But in the proposed version the result seems to be not an option

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL. This however misses the point, if I understand you correctly. The problem is the expression in the proposed code is typed as Iterator whereas the function returns an Option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

filter + next is a find though.

We can simplify the code to Iterator::find + Option::map. That will reduce the number of combinators by one.

We could use a Iterator::find_map, but that would use nested bool::then. But I consider nested combis harder to reason about and I would guess bool::then also is not among commonly used functions out there.

}

/// Consumes this runtime blob and serializes it.
pub fn serialize(self) -> Vec<u8> {
serialize(self.raw_module)
.expect("serializing into a vec should succeed; qed")
}

/// Destructure this structure into the underlying parity-wasm Module.
pub fn into_inner(self) -> RawModule {
self.raw_module
}
}
99 changes: 46 additions & 53 deletions client/executor/src/integration_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,20 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
mod sandbox;

use std::sync::Arc;
use codec::{Encode, Decode};
use hex_literal::hex;
use sp_core::{
blake2_128, blake2_256, ed25519, sr25519, map, Pair,
offchain::{OffchainWorkerExt, OffchainDbExt, testing},
traits::{Externalities, CallInWasm},
traits::Externalities,
};
use sc_runtime_test::wasm_binary_unwrap;
use sp_state_machine::TestExternalities as CoreTestExternalities;
use sp_trie::{TrieConfiguration, trie_types::Layout};
use sp_wasm_interface::HostFunctions as _;
use sp_runtime::traits::BlakeTwo256;
use sc_executor_common::{wasm_runtime::WasmModule, runtime_blob::RuntimeBlob};
use tracing_subscriber::layer::SubscriberExt;

use crate::WasmExecutionMethod;
Expand Down Expand Up @@ -77,13 +79,12 @@ fn call_in_wasm<E: Externalities>(
8,
None,
);
executor.call_in_wasm(
&wasm_binary_unwrap()[..],
None,
executor.uncached_call(
RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
ext,
true,
function,
call_data,
ext,
sp_core::traits::MissingHostFunctions::Allow,
)
}

Expand Down Expand Up @@ -541,28 +542,37 @@ fn should_trap_when_heap_exhausted(wasm_method: WasmExecutionMethod) {
None,
);

let err = executor.call_in_wasm(
&wasm_binary_unwrap()[..],
None,
"test_exhaust_heap",
&[0],
&mut ext.ext(),
sp_core::traits::MissingHostFunctions::Allow,
).unwrap_err();
let err = executor
.uncached_call(
RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
&mut ext.ext(),
true,
"test_exhaust_heap",
&[0],
)
.unwrap_err();

assert!(err.contains("Allocator ran out of space"));
}

test_wasm_execution!(returns_mutable_static);
fn returns_mutable_static(wasm_method: WasmExecutionMethod) {
let runtime = crate::wasm_runtime::create_wasm_runtime_with_code(
fn mk_test_runtime(wasm_method: WasmExecutionMethod, pages: u64) -> Arc<dyn WasmModule> {
let blob = RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..])
.expect("failed to create a runtime blob out of test runtime");

crate::wasm_runtime::create_wasm_runtime_with_code(
wasm_method,
1024,
&wasm_binary_unwrap()[..],
pages,
blob,
HostFunctions::host_functions(),
true,
None,
).expect("Creates runtime");
)
.expect("failed to instantiate wasm runtime")
}

test_wasm_execution!(returns_mutable_static);
fn returns_mutable_static(wasm_method: WasmExecutionMethod) {
let runtime = mk_test_runtime(wasm_method, 1024);

let instance = runtime.new_instance().unwrap();
let res = instance.call_export("returns_mutable_static", &[0]).unwrap();
Expand All @@ -589,14 +599,7 @@ fn restoration_of_globals(wasm_method: WasmExecutionMethod) {
// to our allocator algorithm there are inefficiencies.
const REQUIRED_MEMORY_PAGES: u64 = 32;

let runtime = crate::wasm_runtime::create_wasm_runtime_with_code(
wasm_method,
REQUIRED_MEMORY_PAGES,
&wasm_binary_unwrap()[..],
HostFunctions::host_functions(),
true,
None,
).expect("Creates runtime");
let runtime = mk_test_runtime(wasm_method, REQUIRED_MEMORY_PAGES);
let instance = runtime.new_instance().unwrap();

// On the first invocation we allocate approx. 768KB (75%) of stack and then trap.
Expand All @@ -610,14 +613,7 @@ fn restoration_of_globals(wasm_method: WasmExecutionMethod) {

test_wasm_execution!(interpreted_only heap_is_reset_between_calls);
fn heap_is_reset_between_calls(wasm_method: WasmExecutionMethod) {
let runtime = crate::wasm_runtime::create_wasm_runtime_with_code(
wasm_method,
1024,
&wasm_binary_unwrap()[..],
HostFunctions::host_functions(),
true,
None,
).expect("Creates runtime");
let runtime = mk_test_runtime(wasm_method, 1024);
let instance = runtime.new_instance().unwrap();

let heap_base = instance.get_global_const("__heap_base")
Expand All @@ -642,27 +638,27 @@ fn parallel_execution(wasm_method: WasmExecutionMethod) {
8,
None,
));
let code_hash = blake2_256(wasm_binary_unwrap()).to_vec();
let threads: Vec<_> = (0..8).map(|_|
{
let threads: Vec<_> = (0..8)
.map(|_| {
let executor = executor.clone();
let code_hash = code_hash.clone();
std::thread::spawn(move || {
let mut ext = TestExternalities::default();
let mut ext = ext.ext();
assert_eq!(
executor.call_in_wasm(
&wasm_binary_unwrap()[..],
Some(code_hash.clone()),
"test_twox_128",
&[0],
&mut ext,
sp_core::traits::MissingHostFunctions::Allow,
).unwrap(),
executor
.uncached_call(
RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
&mut ext,
true,
"test_twox_128",
&[0],
)
.unwrap(),
hex!("99e9d85137db46ef4bbea33613baafd5").to_vec().encode(),
);
})
}).collect();
})
.collect();

for t in threads.into_iter() {
t.join().unwrap();
Expand All @@ -671,9 +667,7 @@ fn parallel_execution(wasm_method: WasmExecutionMethod) {

test_wasm_execution!(wasm_tracing_should_work);
fn wasm_tracing_should_work(wasm_method: WasmExecutionMethod) {

use std::sync::{Arc, Mutex};

use std::sync::Mutex;
use sc_tracing::{SpanDatum, TraceEvent};

struct TestTraceHandler(Arc<Mutex<Vec<SpanDatum>>>);
Expand Down Expand Up @@ -779,6 +773,5 @@ fn panic_in_spawned_instance_panics_on_joining_its_result(wasm_method: WasmExecu
&mut ext,
).unwrap_err();

dbg!(&error_result);
assert!(format!("{}", error_result).contains("Spawned task"));
}
26 changes: 15 additions & 11 deletions client/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@ mod wasm_runtime;
mod integration_tests;

pub use wasmi;
pub use native_executor::{with_externalities_safe, NativeExecutor, WasmExecutor, NativeExecutionDispatch};
pub use native_executor::{
with_externalities_safe, NativeExecutor, WasmExecutor, NativeExecutionDispatch,
};
pub use sp_version::{RuntimeVersion, NativeVersion};
pub use codec::Codec;
#[doc(hidden)]
pub use sp_core::traits::{Externalities, CallInWasm};
pub use sp_core::traits::{Externalities};
#[doc(hidden)]
pub use sp_wasm_interface;
pub use wasm_runtime::WasmExecutionMethod;
pub use wasm_runtime::read_embedded_version;

pub use sc_executor_common::{error, sandbox};

Expand All @@ -68,7 +71,7 @@ mod tests {
use sc_runtime_test::wasm_binary_unwrap;
use sp_io::TestExternalities;
use sp_wasm_interface::HostFunctions;
use sp_core::traits::CallInWasm;
use sc_executor_common::runtime_blob::RuntimeBlob;

#[test]
fn call_in_interpreted_wasm_works() {
Expand All @@ -82,14 +85,15 @@ mod tests {
8,
None,
);
let res = executor.call_in_wasm(
&wasm_binary_unwrap()[..],
None,
"test_empty_return",
&[],
&mut ext,
sp_core::traits::MissingHostFunctions::Allow,
).unwrap();
let res = executor
.uncached_call(
RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
&mut ext,
true,
"test_empty_return",
&[],
)
.unwrap();
assert_eq!(res, vec![0u8; 0]);
}
}
Loading