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

wasm-override: Support checking spec_name #10380

Merged
merged 1 commit into from
Dec 3, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 9 additions & 6 deletions client/service/src/client/call_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use std::{cell::RefCell, panic::UnwindSafe, result, sync::Arc};
pub struct LocalCallExecutor<Block: BlockT, B, E> {
backend: Arc<B>,
executor: E,
wasm_override: Option<WasmOverride>,
wasm_override: Arc<Option<WasmOverride>>,
wasm_substitutes: WasmSubstitutes<Block, E, B>,
spawn_handle: Box<dyn SpawnNamed>,
client_config: ClientConfig<Block>,
Expand Down Expand Up @@ -71,7 +71,7 @@ where
Ok(LocalCallExecutor {
backend,
executor,
wasm_override,
wasm_override: Arc::new(wasm_override),
spawn_handle,
client_config,
wasm_substitutes,
Expand All @@ -90,16 +90,19 @@ where
Block: BlockT,
B: backend::Backend<Block>,
{
let spec = self.runtime_version(id)?.spec_version;
let spec = self.runtime_version(id)?;
let code = if let Some(d) = self
.wasm_override
.as_ref()
.map(|o| o.get(&spec, onchain_code.heap_pages))
.as_ref()
.map(|o| o.get(&spec.spec_version, onchain_code.heap_pages, &spec.spec_name))
.flatten()
{
log::debug!(target: "wasm_overrides", "using WASM override for block {}", id);
d
} else if let Some(s) = self.wasm_substitutes.get(spec, onchain_code.heap_pages, id) {
} else if let Some(s) =
self.wasm_substitutes.get(spec.spec_version, onchain_code.heap_pages, id)
{
log::debug!(target: "wasm_substitutes", "Using WASM substitute for block {:?}", id);
s
} else {
Expand Down Expand Up @@ -395,7 +398,7 @@ mod tests {
let call_executor = LocalCallExecutor {
backend: backend.clone(),
executor: executor.clone(),
wasm_override: Some(overrides),
wasm_override: Arc::new(Some(overrides)),
spawn_handle: Box::new(TaskExecutor::new()),
client_config,
wasm_substitutes: WasmSubstitutes::new(
Expand Down
125 changes: 97 additions & 28 deletions client/service/src/client/wasm_override.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,43 @@
//! A custom WASM blob will override on-chain WASM if the spec version matches. If it is
//! required to overrides multiple runtimes, multiple WASM blobs matching each of the spec versions
//! needed must be provided in the given directory.

use sc_executor::RuntimeVersionOf;
use sp_blockchain::Result;
use sp_core::traits::{FetchRuntimeCode, RuntimeCode};
use sp_core::traits::{FetchRuntimeCode, RuntimeCode, WrappedRuntimeCode};
use sp_state_machine::BasicExternalities;
use sp_version::RuntimeVersion;
use std::{
collections::{hash_map::DefaultHasher, HashMap},
fs,
hash::Hasher as _,
path::{Path, PathBuf},
time::{Duration, Instant},
};

#[derive(Clone, Debug, PartialEq)]
/// The interval in that we will print a warning when a wasm blob `spec_name`
/// doesn't match with the on-chain `spec_name`.
const WARN_INTERVAL: Duration = Duration::from_secs(30);
Copy link
Contributor

@kianenigma kianenigma Nov 30, 2021

Choose a reason for hiding this comment

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

wouldn't it be much easier if we just abort the command if this didn't match? I would continuously print a warning if at least the wasm-override was applied, and now you want to warn the user. If it was not even applied, I would either exit, or warn once about it and move on.

Copy link
Contributor

Choose a reason for hiding this comment

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

it would spare you most of the Arc/Mutex layers as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

wouldn't it be much easier if we just abort the command if this didn't match?

Which command?

I mean the best would be to do that at startup, but there we don't know the spec name and that is also legal to change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Printing just one warning, will probably not bring that much, as it can be overseen relative easily.


/// Auxiliary structure that holds a wasm blob and its hash.
#[derive(Debug)]
struct WasmBlob {
/// The actual wasm blob, aka the code.
code: Vec<u8>,
/// The hash of [`Self::code`].
hash: Vec<u8>,
/// The path where this blob was found.
path: PathBuf,
/// The `spec_name` found in the runtime version of this blob.
spec_name: String,
/// When was the last time we have warned about the wasm blob having
/// a wrong `spec_name`?
last_warn: parking_lot::Mutex<Option<Instant>>,
}

impl WasmBlob {
fn new(code: Vec<u8>) -> Self {
let hash = make_hash(&code);
Self { code, hash }
fn new(code: Vec<u8>, hash: Vec<u8>, path: PathBuf, spec_name: String) -> Self {
Self { code, hash, path, spec_name, last_warn: Default::default() }
}

fn runtime_code(&self, heap_pages: Option<u64>) -> RuntimeCode {
Expand Down Expand Up @@ -103,7 +117,7 @@ impl From<WasmOverrideError> for sp_blockchain::Error {

/// Scrapes WASM from a folder and returns WASM from that folder
/// if the runtime spec version matches.
#[derive(Clone, Debug)]
#[derive(Debug)]
pub struct WasmOverride {
// Map of runtime spec version -> Wasm Blob
overrides: HashMap<u32, WasmBlob>,
Expand All @@ -122,8 +136,35 @@ impl WasmOverride {
/// Gets an override by it's runtime spec version.
///
/// Returns `None` if an override for a spec version does not exist.
pub fn get<'a, 'b: 'a>(&'b self, spec: &u32, pages: Option<u64>) -> Option<RuntimeCode<'a>> {
self.overrides.get(spec).map(|w| w.runtime_code(pages))
pub fn get<'a, 'b: 'a>(
&'b self,
spec: &u32,
pages: Option<u64>,
spec_name: &str,
) -> Option<RuntimeCode<'a>> {
self.overrides.get(spec).and_then(|w| {
if spec_name == w.spec_name {
Some(w.runtime_code(pages))
} else {
let mut last_warn = w.last_warn.lock();
let now = Instant::now();

if last_warn.map_or(true, |l| l + WARN_INTERVAL <= now) {
*last_warn = Some(now);

tracing::warn!(
target = "wasm_overrides",
on_chain_spec_name = %spec_name,
override_spec_name = %w.spec_name,
spec_version = %spec,
wasm_file = %w.path.display(),
"On chain and override `spec_name` do not match! Ignoring override.",
);
}

None
}
})
}

/// Scrapes a folder for WASM runtimes.
Expand All @@ -147,22 +188,29 @@ impl WasmOverride {
let path = entry.path();
match path.extension().map(|e| e.to_str()).flatten() {
Some("wasm") => {
let wasm = WasmBlob::new(fs::read(&path).map_err(handle_err)?);
let version = Self::runtime_version(executor, &wasm, Some(128))?;
log::info!(
let code = fs::read(&path).map_err(handle_err)?;
let code_hash = make_hash(&code);
let version = Self::runtime_version(executor, &code, &code_hash, Some(128))?;

tracing::info!(
target: "wasm_overrides",
"Found wasm override in file: `{:?}`, version: {}",
path.to_str(),
version,
version = %version,
file = %path.display(),
"Found wasm override.",
);
if let Some(_duplicate) = overrides.insert(version.spec_version, wasm) {
log::info!(

let wasm =
WasmBlob::new(code, code_hash, path.clone(), version.spec_name.to_string());

if let Some(other) = overrides.insert(version.spec_version, wasm) {
tracing::info!(
target: "wasm_overrides",
"Found duplicate spec version for runtime in file: `{:?}`, version: {}",
path.to_str(),
version,
first = %other.path.display(),
second = %path.display(),
%version,
"Found duplicate spec version for runtime.",
);
duplicates.push(format!("{}", path.display()));
duplicates.push(path.display().to_string());
}
},
_ => (),
Expand All @@ -178,15 +226,23 @@ impl WasmOverride {

fn runtime_version<E>(
executor: &E,
code: &WasmBlob,
code: &[u8],
code_hash: &[u8],
heap_pages: Option<u64>,
) -> Result<RuntimeVersion>
where
E: RuntimeVersionOf,
{
let mut ext = BasicExternalities::default();
executor
.runtime_version(&mut ext, &code.runtime_code(heap_pages))
.runtime_version(
&mut ext,
&RuntimeCode {
code_fetcher: &WrappedRuntimeCode(code.into()),
heap_pages,
hash: code_hash.into(),
},
)
.map_err(|e| WasmOverrideError::VersionInvalid(format!("{:?}", e)).into())
}
}
Expand All @@ -195,9 +251,18 @@ impl WasmOverride {
#[cfg(test)]
pub fn dummy_overrides() -> WasmOverride {
let mut overrides = HashMap::new();
overrides.insert(0, WasmBlob::new(vec![0, 0, 0, 0, 0, 0, 0, 0]));
overrides.insert(1, WasmBlob::new(vec![1, 1, 1, 1, 1, 1, 1, 1]));
overrides.insert(2, WasmBlob::new(vec![2, 2, 2, 2, 2, 2, 2, 2]));
overrides.insert(
0,
WasmBlob::new(vec![0, 0, 0, 0, 0, 0, 0, 0], vec![0], PathBuf::new(), "test".into()),
);
overrides.insert(
1,
WasmBlob::new(vec![1, 1, 1, 1, 1, 1, 1, 1], vec![1], PathBuf::new(), "test".into()),
);
overrides.insert(
2,
WasmBlob::new(vec![2, 2, 2, 2, 2, 2, 2, 2], vec![2], PathBuf::new(), "test".into()),
);
WasmOverride { overrides }
}

Expand Down Expand Up @@ -226,15 +291,19 @@ mod tests {

#[test]
fn should_get_runtime_version() {
let wasm = WasmBlob::new(substrate_test_runtime::wasm_binary_unwrap().to_vec());
let executor = NativeElseWasmExecutor::<LocalExecutorDispatch>::new(
WasmExecutionMethod::Interpreted,
Some(128),
1,
);

let version = WasmOverride::runtime_version(&executor, &wasm, Some(128))
.expect("should get the `RuntimeVersion` of the test-runtime wasm blob");
let version = WasmOverride::runtime_version(
&executor,
substrate_test_runtime::wasm_binary_unwrap(),
&[1],
Some(128),
)
.expect("should get the `RuntimeVersion` of the test-runtime wasm blob");
assert_eq!(version.spec_version, 2);
}

Expand Down
12 changes: 12 additions & 0 deletions primitives/runtime/src/runtime_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ impl AsRef<[u8]> for RuntimeString {
}
}

#[cfg(feature = "std")]
impl std::ops::Deref for RuntimeString {
type Target = str;

fn deref(&self) -> &str {
match self {
Self::Borrowed(val) => &val,
Self::Owned(val) => &val,
}
}
}

impl Encode for RuntimeString {
fn encode(&self) -> Vec<u8> {
match self {
Expand Down