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

WASM Local-blob override #7317

Merged
merged 72 commits into from
Oct 26, 2020
Merged
Show file tree
Hide file tree
Changes from 66 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
aa4a698
Provide WASM overwrite functionality in LocalCallExecutor
Oct 13, 2020
2119c33
Merge branch 'master' of github.com:paritytech/substrate into master
Oct 13, 2020
ad6a811
formatting
Oct 13, 2020
087e797
Make comment clearer
Oct 13, 2020
77134e1
comments
Oct 13, 2020
f030c7e
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 14, 2020
770d825
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 14, 2020
a54ae38
Fix spaces, remove call into backend for 'heap_pages' in 'try_replace'
Oct 14, 2020
0cf1263
apply feedback
Oct 14, 2020
c67b981
Error if path is not a directory, Comments,
Oct 15, 2020
7a83168
make WasmOverwrite Option<>
Oct 15, 2020
b04a260
Change to one CLI argument for overwrites
Oct 15, 2020
7704458
change unwrap() to expect()
Oct 15, 2020
f80b33c
comment
Oct 15, 2020
d1254c5
Remove `check_overwrites`
Oct 16, 2020
ed19163
Merge branch 'master' of github.com:paritytech/substrate into insipx/…
Oct 16, 2020
3feed25
Encapsulate checking for overwrites in LocalCallExecutor
Oct 16, 2020
bdd847d
move duplicate code into function
Oct 19, 2020
73efeb6
Update client/cli/src/params/import_params.rs
insipx Oct 19, 2020
2675653
Merge branch 'insipx/wasm-local' of github.com:paritytech/substrate i…
Oct 19, 2020
474c442
comma
Oct 19, 2020
8a51780
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 19, 2020
a9f8e85
Merge branch 'insipx/wasm-local' of github.com:paritytech/substrate i…
Oct 19, 2020
2aab45e
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 19, 2020
cd7a3e3
Merge branch 'insipx/wasm-local' of github.com:paritytech/substrate i…
Oct 19, 2020
2895bc4
cache hash in WasmBlob
Oct 19, 2020
f9fac2b
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 19, 2020
0e4e972
Merge branch 'insipx/wasm-local' of github.com:paritytech/substrate i…
Oct 19, 2020
abc48f7
Update client/service/src/client/client.rs
insipx Oct 19, 2020
8f4c857
move getting overwrite into its own function
Oct 19, 2020
32bd6db
fix error when directory is not a directory
Oct 19, 2020
6a50eee
Merge branch 'insipx/wasm-local' of github.com:paritytech/substrate i…
Oct 19, 2020
a0c1f55
Error on duplicate WASM runtimes
Oct 19, 2020
f940c71
better comment, grammar
Oct 19, 2020
26975d3
docs
Oct 19, 2020
8433731
Revert StateBackend back to _
Oct 19, 2020
3e8d18f
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 20, 2020
d14e1d7
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 20, 2020
7b99336
Update client/service/src/client/call_executor.rs
insipx Oct 20, 2020
626e453
Add two tests, fix doc comments
Oct 21, 2020
fed565c
remove redundant `Return` from expect msg
Oct 21, 2020
5812b10
Update client/cli/src/params/import_params.rs
insipx Oct 22, 2020
d40d5bb
Update client/service/src/client/call_executor.rs
insipx Oct 22, 2020
693f506
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 22, 2020
a2a41de
Update client/service/src/config.rs
insipx Oct 22, 2020
824b24e
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 22, 2020
0506100
Add Module Documentation, match on '.wasm' extension
Oct 22, 2020
82e2683
merge suggestions
Oct 22, 2020
554f68e
Add test for scraping WASM blob
Oct 22, 2020
2ddf6a0
fix expect
Oct 22, 2020
18589c7
remove creating another block in LocalCallExecutor test
Oct 22, 2020
5781c1e
remove unused import
Oct 22, 2020
1401b18
add tests for duplicates and scraping wasm
Oct 22, 2020
37266bf
make tests a bit nicer
Oct 22, 2020
1ef18ab
add test for ignoring non-.wasm files
Oct 22, 2020
551f3ed
check error message in test
Oct 23, 2020
54969dc
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 23, 2020
25dc83f
Merge branch 'insipx/wasm-local' of github.com:paritytech/substrate i…
Oct 23, 2020
7d3ee25
remove println
Oct 23, 2020
e654dbe
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 23, 2020
665e20c
make tests prettier
Oct 23, 2020
5a91f0d
Update client/service/src/client/wasm_overwrite.rs
insipx Oct 23, 2020
b2b9ba5
comment for seemingly random client
Oct 23, 2020
103f149
locally-built -> custom
Oct 23, 2020
a84dcbf
remove unused import
Oct 23, 2020
1ac97cb
fix comment
Oct 23, 2020
6cc0a8f
Merge branch 'master' of github.com:paritytech/substrate into insipx/…
insipx Oct 23, 2020
8503689
Merge branch 'master' of github.com:paritytech/substrate into insipx/…
Oct 25, 2020
8fc367d
Merge branch 'insipx/wasm-local' of github.com:paritytech/substrate i…
Oct 25, 2020
f90db2f
rename all references to overwrite with override
Oct 26, 2020
1b355b7
fix cli flag in module documentation
Oct 26, 2020
427d9ae
Merge branch 'master' of github.com:paritytech/substrate into insipx/…
Oct 26, 2020
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
1 change: 1 addition & 0 deletions Cargo.lock

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

10 changes: 10 additions & 0 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,15 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
.unwrap_or_default())
}

/// Get the path where WASM overwrites live.
///
/// By default this is `None`.
fn wasm_runtime_overwrites(&self) -> Option<PathBuf> {
self.import_params()
.map(|x| x.wasm_runtime_overwrites())
.unwrap_or_default()
}

/// Get the execution strategies.
///
/// By default this is retrieved from `ImportParams` if it is available. Otherwise its
Expand Down Expand Up @@ -492,6 +501,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
state_cache_child_ratio: self.state_cache_child_ratio()?,
pruning: self.pruning(unsafe_pruning, &role)?,
wasm_method: self.wasm_method()?,
wasm_runtime_overwrites: self.wasm_runtime_overwrites(),
execution_strategies: self.execution_strategies(is_dev, is_validator)?,
rpc_http: self.rpc_http(DCV::rpc_http_listen_port())?,
rpc_ws: self.rpc_ws(DCV::rpc_ws_listen_port())?,
Expand Down
13 changes: 13 additions & 0 deletions client/cli/src/params/import_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::params::DatabaseParams;
use crate::params::PruningParams;
use sc_client_api::execution_extensions::ExecutionStrategies;
use structopt::StructOpt;
use std::path::PathBuf;

/// Parameters for block import.
#[derive(Debug, StructOpt)]
Expand Down Expand Up @@ -55,6 +56,12 @@ pub struct ImportParams {
)]
pub wasm_method: WasmExecutionMethod,

/// Specify the path where local WASM runtimes are stored.
///
/// These runtimes will override on-chain runtimes when the version matches.
insipx marked this conversation as resolved.
Show resolved Hide resolved
#[structopt(long, value_name = "PATH", parse(from_os_str))]
pub wasm_runtime_overwrites: Option<PathBuf>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a terminology query - should we use override vs overwrite everywhere? Seems like overwrite is used predominantly but I think override is a better choice - either way good to be consistent, unless actually there is some difference in meaning - in which case perhaps choose another word, so that the two concepts are more obviously distinct in their name.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like override better too (and may be the one to blame for the inconsistent docs here, not sure) but kept my mouth shut for fear of nitpicking, but as you opened up the can... :)

"Overwrite" is more concrete than "override" and in some sense we are writing something over what was previously there. We're "clobbering" the on-chain runtime. On the other hand "override" reflects more on what the actual outcome of the operation is: taking control over the normal flow of a process (picking the runtime) and intervene. It's very close to the dictionary definition of "override": "interrupt the action of (an automatic device), typically in order to take manual control.".

Either works. I like "override" better. Andrews call.


#[allow(missing_docs)]
#[structopt(flatten)]
pub execution_strategies: ExecutionStrategiesParams,
Expand Down Expand Up @@ -103,6 +110,12 @@ impl ImportParams {
self.wasm_method.into()
}

/// Enable overwriting on-chain WASM with locally-stored WASM
/// by specifying the path where local WASM is stored.
pub fn wasm_runtime_overwrites(&self) -> Option<PathBuf> {
self.wasm_runtime_overwrites.clone()
}

/// Get execution strategies for the parameters
pub fn execution_strategies(&self, is_dev: bool, is_validator: bool) -> ExecutionStrategies {
let exec = &self.execution_strategies;
Expand Down
1 change: 1 addition & 0 deletions client/service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ directories = "2.0.2"

[dev-dependencies]
substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" }
substrate-test-runtime = { versino = "2.0.0", path = "../../test-utils/runtime/" }
sp-consensus-babe = { version = "0.8.0", path = "../../primitives/consensus/babe" }
grandpa = { version = "0.8.0", package = "sc-finality-grandpa", path = "../finality-grandpa" }
grandpa-primitives = { version = "2.0.0", package = "sp-finality-grandpa", path = "../../primitives/finality-grandpa" }
Expand Down
5 changes: 3 additions & 2 deletions client/service/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,9 @@ pub fn new_full_parts<TBl, TRtApi, TExecDisp>(
Box::new(task_manager.spawn_handle()),
config.prometheus_config.as_ref().map(|config| config.registry.clone()),
ClientConfig {
offchain_worker_enabled : config.offchain_worker.enabled ,
offchain_worker_enabled : config.offchain_worker.enabled,
offchain_indexing_api: config.offchain_worker.indexing_enabled,
wasm_runtime_overwrites: config.wasm_runtime_overwrites.clone(),
},
)?
};
Expand Down Expand Up @@ -396,7 +397,7 @@ pub fn new_client<E, Block, RA>(
const CANONICALIZATION_DELAY: u64 = 4096;

let backend = Arc::new(Backend::new(settings, CANONICALIZATION_DELAY)?);
let executor = crate::client::LocalCallExecutor::new(backend.clone(), executor, spawn_handle, config.clone());
let executor = crate::client::LocalCallExecutor::new(backend.clone(), executor, spawn_handle, config.clone())?;
Ok((
crate::client::Client::new(
backend.clone(),
Expand Down
120 changes: 111 additions & 9 deletions client/service/src/client/call_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,36 +28,71 @@ use sp_state_machine::{
use sc_executor::{RuntimeVersion, RuntimeInfo, NativeVersion};
use sp_externalities::Extensions;
use sp_core::{
NativeOrEncoded, NeverNativeValue, traits::{CodeExecutor, SpawnNamed},
NativeOrEncoded, NeverNativeValue, traits::{CodeExecutor, SpawnNamed, RuntimeCode},
offchain::storage::OffchainOverlayedChanges,
};
use sp_api::{ProofRecorder, InitializeBlock, StorageTransactionCache};
use sc_client_api::{backend, call_executor::CallExecutor};
use super::client::ClientConfig;
use super::{client::ClientConfig, wasm_overwrite::WasmOverwrite};

/// Call executor that executes methods locally, querying all required
/// data from local backend.
pub struct LocalCallExecutor<B, E> {
backend: Arc<B>,
executor: E,
wasm_overwrite: Option<WasmOverwrite<E>>,
spawn_handle: Box<dyn SpawnNamed>,
client_config: ClientConfig,
}

impl<B, E> LocalCallExecutor<B, E> {
impl<B, E> LocalCallExecutor<B, E>
where
E: CodeExecutor + RuntimeInfo + Clone + 'static
{
/// Creates new instance of local call executor.
pub fn new(
backend: Arc<B>,
executor: E,
spawn_handle: Box<dyn SpawnNamed>,
client_config: ClientConfig,
) -> Self {
LocalCallExecutor {
) -> sp_blockchain::Result<Self> {
let wasm_overwrite = client_config.wasm_runtime_overwrites
.as_ref()
.map(|p| WasmOverwrite::new(p.clone(), executor.clone()))
.transpose()?;

Ok(LocalCallExecutor {
backend,
executor,
wasm_overwrite,
spawn_handle,
client_config,
}
})
}

/// Check if local runtime code overrides are enabled and one is available
/// for the given `BlockId`. If yes, return it; otherwise return the same
/// `RuntimeCode` instance that was passed.
fn check_overwrite<'a, Block>(
&'a self,
onchain_code: RuntimeCode<'a>,
id: &BlockId<Block>,
) -> sp_blockchain::Result<RuntimeCode<'a>>
where
Block: BlockT,
B: backend::Backend<Block>,
{
let code = self.wasm_overwrite
.as_ref()
.map::<sp_blockchain::Result<Option<RuntimeCode>>, _>(|o| {
let spec = self.runtime_version(id)?.spec_version;
Ok(o.get(&spec, onchain_code.heap_pages))
})
.transpose()?
.flatten()
.unwrap_or(onchain_code);

Ok(code)
}
}

Expand All @@ -66,6 +101,7 @@ impl<B, E> Clone for LocalCallExecutor<B, E> where E: Clone {
LocalCallExecutor {
backend: self.backend.clone(),
executor: self.executor.clone(),
wasm_overwrite: self.wasm_overwrite.clone(),
spawn_handle: self.spawn_handle.clone(),
client_config: self.client_config.clone(),
}
Expand Down Expand Up @@ -101,6 +137,8 @@ where
)?;
let state = self.backend.state_at(*id)?;
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
let runtime_code = self.check_overwrite(state_runtime_code.runtime_code()?, id)?;

let return_data = StateMachine::new(
&state,
changes_trie,
Expand All @@ -110,7 +148,7 @@ where
method,
call_data,
extensions.unwrap_or_default(),
&state_runtime_code.runtime_code()?,
&runtime_code,
self.spawn_handle.clone(),
).execute_using_consensus_failure_handler::<_, NeverNativeValue, fn() -> _>(
strategy.get_manager(),
Expand Down Expand Up @@ -173,7 +211,7 @@ where
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&trie_state);
// It is important to extract the runtime code here before we create the proof
// recorder.
let runtime_code = state_runtime_code.runtime_code()?;
let runtime_code = self.check_overwrite(state_runtime_code.runtime_code()?, at)?;

let backend = sp_state_machine::ProvingBackend::new_with_recorder(
trie_state,
Expand All @@ -198,7 +236,8 @@ where
},
None => {
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
let runtime_code = state_runtime_code.runtime_code()?;
let runtime_code = self.check_overwrite(state_runtime_code.runtime_code()?, at)?;

let mut state_machine = StateMachine::new(
&state,
changes_trie_state,
Expand Down Expand Up @@ -279,3 +318,66 @@ impl<B, E, Block> sp_version::GetRuntimeVersion<Block> for LocalCallExecutor<B,
CallExecutor::runtime_version(self, at).map_err(|e| format!("{:?}", e))
}
}

#[cfg(test)]
mod tests {
use super::*;
use substrate_test_runtime_client::{LocalExecutor, GenesisInit, runtime};
use sc_executor::{NativeExecutor, WasmExecutionMethod};
use sp_core::{traits::{WrappedRuntimeCode, FetchRuntimeCode}, testing::TaskExecutor};
use sc_client_api::in_mem;

#[test]
fn should_get_overwrite_if_exists() {
let executor =
NativeExecutor::<LocalExecutor>::new(WasmExecutionMethod::Interpreted, Some(128), 1);

let overwrites = crate::client::wasm_overwrite::dummy_overwrites(&executor);
let onchain_code = WrappedRuntimeCode(substrate_test_runtime::wasm_binary_unwrap().into());
let onchain_code = RuntimeCode {
code_fetcher: &onchain_code,
heap_pages: Some(128),
hash: vec![0, 0, 0, 0],
};

let backend = Arc::new(in_mem::Backend::<runtime::Block>::new());

// wasm_runtime_overwrites is `None` here because we construct the
// LocalCallExecutor directly later on
let client_config = ClientConfig {
offchain_worker_enabled: false,
offchain_indexing_api: false,
wasm_runtime_overwrites: None,
};

// client is used for the convenience of creating and inserting the genesis block.
let _client = substrate_test_runtime_client::client::new_with_backend::<
insipx marked this conversation as resolved.
Show resolved Hide resolved
_,
_,
runtime::Block,
_,
runtime::RuntimeApi,
>(
backend.clone(),
executor.clone(),
&substrate_test_runtime_client::GenesisParameters::default().genesis_storage(),
None,
Box::new(TaskExecutor::new()),
None,
Default::default(),
).expect("Creates a client");

let call_executor = LocalCallExecutor {
backend: backend.clone(),
executor,
wasm_overwrite: Some(overwrites),
spawn_handle: Box::new(TaskExecutor::new()),
client_config,
};

let check = call_executor.check_overwrite(onchain_code, &BlockId::Number(Default::default()))
.expect("RuntimeCode overwrite");

assert_eq!(Some(vec![2, 2, 2, 2, 2, 2, 2, 2]), check.fetch_runtime_code().map(Into::into));
}
}
5 changes: 4 additions & 1 deletion client/service/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use std::{
marker::PhantomData,
collections::{HashSet, BTreeMap, HashMap},
sync::Arc, panic::UnwindSafe, result,
path::PathBuf
};
use log::{info, trace, warn};
use parking_lot::{Mutex, RwLock};
Expand Down Expand Up @@ -181,6 +182,8 @@ pub struct ClientConfig {
pub offchain_worker_enabled: bool,
/// If true, allows access from the runtime to write into offchain worker db.
pub offchain_indexing_api: bool,
/// Path where WASM files exist to overwrite the on-chain WASM.
pub wasm_runtime_overwrites: Option<PathBuf>,
}

/// Create a client with the explicitly provided backend.
Expand All @@ -201,7 +204,7 @@ pub fn new_with_backend<B, E, Block, S, RA>(
Block: BlockT,
B: backend::LocalBackend<Block> + 'static,
{
let call_executor = LocalCallExecutor::new(backend.clone(), executor, spawn_handle, config.clone());
let call_executor = LocalCallExecutor::new(backend.clone(), executor, spawn_handle, config.clone())?;
let extensions = ExecutionExtensions::new(Default::default(), keystore);
Client::new(
backend,
Expand Down
2 changes: 1 addition & 1 deletion client/service/src/client/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub fn new_light<B, S, RA, E>(
code_executor,
spawn_handle.clone(),
ClientConfig::default()
);
)?;
let executor = GenesisCallExecutor::new(backend.clone(), local_executor);
Client::new(
backend,
Expand Down
1 change: 1 addition & 0 deletions client/service/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub mod light;
mod call_executor;
mod client;
mod block_rules;
mod wasm_overwrite;

pub use self::{
call_executor::LocalCallExecutor,
Expand Down
Loading