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

Deprecate NativeElseWasmExecutor #4329

Merged
merged 8 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
26 changes: 2 additions & 24 deletions cumulus/test/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,33 +45,11 @@ pub use substrate_test_client::*;

pub type ParachainBlockData = cumulus_primitives_core::ParachainBlockData<Block>;

mod local_executor {
/// Native executor instance.
pub struct LocalExecutor;

impl sc_executor::NativeExecutionDispatch for LocalExecutor {
type ExtendHostFunctions =
cumulus_primitives_proof_size_hostfunction::storage_proof_size::HostFunctions;

fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
cumulus_test_runtime::api::dispatch(method, data)
}

fn native_version() -> sc_executor::NativeVersion {
cumulus_test_runtime::native_version()
}
}
}

/// Native executor used for tests.
pub use local_executor::LocalExecutor;

/// Test client database backend.
pub type Backend = substrate_test_client::Backend<Block>;

/// Test client executor.
pub type Executor =
client::LocalCallExecutor<Block, Backend, sc_executor::NativeElseWasmExecutor<LocalExecutor>>;
pub type Executor = client::LocalCallExecutor<Block, Backend, WasmExecutor>;

/// Test client builder for Cumulus
pub type TestClientBuilder =
Expand Down Expand Up @@ -100,7 +78,7 @@ impl substrate_test_client::GenesisInit for GenesisParameters {
}
}

/// A `test-runtime` extensions to `TestClientBuilder`.
/// A `test-runtime` extensions to [`TestClientBuilder`].
pub trait TestClientBuilderExt: Sized {
/// Build the test client.
fn build(self) -> Client {
Expand Down
28 changes: 22 additions & 6 deletions substrate/client/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fn unwrap_heap_pages(pages: Option<HeapAllocStrategy>) -> HeapAllocStrategy {
}

/// Builder for creating a [`WasmExecutor`] instance.
pub struct WasmExecutorBuilder<H> {
pub struct WasmExecutorBuilder<H = sp_io::SubstrateHostFunctions> {
_phantom: PhantomData<H>,
method: WasmExecutionMethod,
onchain_heap_alloc_strategy: Option<HeapAllocStrategy>,
Expand Down Expand Up @@ -218,7 +218,7 @@ impl<H> WasmExecutorBuilder<H> {

/// An abstraction over Wasm code executor. Supports selecting execution backend and
/// manages runtime cache.
pub struct WasmExecutor<H> {
pub struct WasmExecutor<H = sp_io::SubstrateHostFunctions> {
/// Method used to execute fallback Wasm code.
method: WasmExecutionMethod,
/// The heap allocation strategy for onchain Wasm calls.
Expand Down Expand Up @@ -252,10 +252,13 @@ impl<H> Clone for WasmExecutor<H> {
}
}

impl<H> WasmExecutor<H>
where
H: HostFunctions,
{
impl Default for WasmExecutor<sp_io::SubstrateHostFunctions> {
fn default() -> Self {
WasmExecutorBuilder::new().build()
}
}

impl<H> WasmExecutor<H> {
/// Create new instance.
///
/// # Parameters
Expand Down Expand Up @@ -312,7 +315,12 @@ where
pub fn allow_missing_host_functions(&mut self, allow_missing_host_functions: bool) {
self.allow_missing_host_functions = allow_missing_host_functions
}
}

impl<H> WasmExecutor<H>
where
H: HostFunctions,
{
/// Execute the given closure `f` with the latest runtime (based on `runtime_code`).
///
/// The closure `f` is expected to return `Err(_)` when there happened a `panic!` in native code
Expand Down Expand Up @@ -558,6 +566,7 @@ where

/// A generic `CodeExecutor` implementation that uses a delegate to determine wasm code equivalence
/// and dispatch to native code when possible, falling back on `WasmExecutor` when not.
#[deprecated(note = "Native execution will be deprecated, please replace with `WasmExecutor`")]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we indicate estimated removal date?

pub struct NativeElseWasmExecutor<D: NativeExecutionDispatch> {
/// Native runtime version info.
native_version: NativeVersion,
Expand All @@ -568,6 +577,7 @@ pub struct NativeElseWasmExecutor<D: NativeExecutionDispatch> {
use_native: bool,
}

#[allow(deprecated)]
impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
///
/// Create new instance.
Expand Down Expand Up @@ -628,6 +638,7 @@ impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
}
}

#[allow(deprecated)]
impl<D: NativeExecutionDispatch> RuntimeVersionOf for NativeElseWasmExecutor<D> {
fn runtime_version(
&self,
Expand All @@ -638,12 +649,14 @@ impl<D: NativeExecutionDispatch> RuntimeVersionOf for NativeElseWasmExecutor<D>
}
}

#[allow(deprecated)]
impl<D: NativeExecutionDispatch> GetNativeVersion for NativeElseWasmExecutor<D> {
fn native_version(&self) -> &NativeVersion {
&self.native_version
}
}

#[allow(deprecated)]
impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecutor<D> {
type Error = Error;

Expand Down Expand Up @@ -718,6 +731,7 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecut
}
}

#[allow(deprecated)]
impl<D: NativeExecutionDispatch> Clone for NativeElseWasmExecutor<D> {
fn clone(&self) -> Self {
NativeElseWasmExecutor {
Expand All @@ -728,6 +742,7 @@ impl<D: NativeExecutionDispatch> Clone for NativeElseWasmExecutor<D> {
}
}

#[allow(deprecated)]
impl<D: NativeExecutionDispatch> sp_core::traits::ReadRuntimeVersion for NativeElseWasmExecutor<D> {
fn read_runtime_version(
&self,
Expand Down Expand Up @@ -765,6 +780,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn native_executor_registers_custom_interface() {
let executor = NativeElseWasmExecutor::<MyExecutorDispatch>::new_with_wasm_executor(
WasmExecutor::builder().build(),
Expand Down
10 changes: 4 additions & 6 deletions substrate/client/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,16 @@ mod executor;
mod integration_tests;
mod wasm_runtime;

pub use self::{
executor::{
with_externalities_safe, NativeElseWasmExecutor, NativeExecutionDispatch, WasmExecutor,
},
wasm_runtime::{read_embedded_version, WasmExecutionMethod},
};
pub use codec::Codec;
#[allow(deprecated)]
pub use executor::NativeElseWasmExecutor;
pub use executor::{with_externalities_safe, NativeExecutionDispatch, WasmExecutor};
#[doc(hidden)]
pub use sp_core::traits::Externalities;
pub use sp_version::{NativeVersion, RuntimeVersion};
#[doc(hidden)]
pub use sp_wasm_interface;
pub use wasm_runtime::{read_embedded_version, WasmExecutionMethod};

pub use sc_executor_common::{
error,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ mod tests {
Arc<Client<sc_client_api::in_mem::Backend<Block>>>,
) {
let backend = Arc::new(sc_client_api::in_mem::Backend::new());
let executor = substrate_test_runtime_client::new_native_or_wasm_executor();
let executor = substrate_test_runtime_client::WasmExecutor::default();
let client_config = sc_service::ClientConfig::default();
let genesis_block_builder = sc_service::GenesisBlockBuilder::new(
&substrate_test_runtime_client::GenesisParameters::default().genesis_storage(),
Expand Down
6 changes: 2 additions & 4 deletions substrate/client/rpc-spec-v2/src/chain_head/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use super::*;
use crate::{
chain_head::{api::ChainHeadApiClient, event::MethodResponse, test_utils::ChainHeadMockClient},
common::events::{StorageQuery, StorageQueryType, StorageResultType},
hex_string,
};

use super::*;
use assert_matches::assert_matches;
use codec::{Decode, Encode};
use futures::Future;
Expand All @@ -32,7 +31,6 @@ use jsonrpsee::{
},
rpc_params, MethodsError as Error, RpcModule,
};

use sc_block_builder::BlockBuilderBuilder;
use sc_client_api::ChildInfo;
use sc_service::client::new_in_mem;
Expand Down Expand Up @@ -2550,7 +2548,7 @@ async fn follow_report_multiple_pruned_block() {
async fn pin_block_references() {
// Manually construct an in-memory backend and client.
let backend = Arc::new(sc_client_api::in_mem::Backend::new());
let executor = substrate_test_runtime_client::new_native_or_wasm_executor();
let executor = substrate_test_runtime_client::WasmExecutor::default();
let client_config = sc_service::ClientConfig::default();

let genesis_block_builder = sc_service::GenesisBlockBuilder::new(
Expand Down
11 changes: 7 additions & 4 deletions substrate/client/service/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ use sc_client_api::{
use sc_client_db::{Backend, DatabaseSettings};
use sc_consensus::import_queue::ImportQueue;
use sc_executor::{
sp_wasm_interface::HostFunctions, HeapAllocStrategy, NativeElseWasmExecutor,
NativeExecutionDispatch, RuntimeVersionOf, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY,
sp_wasm_interface::HostFunctions, HeapAllocStrategy, NativeExecutionDispatch, RuntimeVersionOf,
WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY,
};
use sc_keystore::LocalKeystore;
use sc_network::{
Expand Down Expand Up @@ -263,10 +263,13 @@ where
}

/// Creates a [`NativeElseWasmExecutor`] according to [`Configuration`].
#[deprecated(note = "Please switch to `new_wasm_executor`.")]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the same - removal date could be given.

#[allow(deprecated)]
pub fn new_native_or_wasm_executor<D: NativeExecutionDispatch>(
config: &Configuration,
) -> NativeElseWasmExecutor<D> {
NativeElseWasmExecutor::new_with_wasm_executor(new_wasm_executor(config))
) -> sc_executor::NativeElseWasmExecutor<D> {
#[allow(deprecated)]
sc_executor::NativeElseWasmExecutor::new_with_wasm_executor(new_wasm_executor(config))
}

/// Creates a [`WasmExecutor`] according to [`Configuration`].
Expand Down
17 changes: 4 additions & 13 deletions substrate/client/service/src/client/call_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,26 +337,17 @@ mod tests {
use super::*;
use backend::Backend;
use sc_client_api::in_mem;
use sc_executor::{NativeElseWasmExecutor, WasmExecutor};
use sc_executor::WasmExecutor;
use sp_core::{
testing::TaskExecutor,
traits::{FetchRuntimeCode, WrappedRuntimeCode},
};
use std::collections::HashMap;
use substrate_test_runtime_client::{runtime, GenesisInit, LocalExecutorDispatch};

fn executor() -> NativeElseWasmExecutor<LocalExecutorDispatch> {
NativeElseWasmExecutor::new_with_wasm_executor(
WasmExecutor::builder()
.with_max_runtime_instances(1)
.with_runtime_cache_size(2)
.build(),
)
}
use substrate_test_runtime_client::{runtime, GenesisInit};

#[test]
fn should_get_override_if_exists() {
let executor = executor();
let executor = WasmExecutor::default();

let overrides = crate::client::wasm_override::dummy_overrides();
let onchain_code = WrappedRuntimeCode(substrate_test_runtime::wasm_binary_unwrap().into());
Expand Down Expand Up @@ -425,7 +416,7 @@ mod tests {
fn returns_runtime_version_from_substitute() {
const SUBSTITUTE_SPEC_NAME: &str = "substitute-spec-name-cool";

let executor = executor();
let executor = WasmExecutor::default();

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

Expand Down
23 changes: 10 additions & 13 deletions substrate/client/service/src/client/wasm_override.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,24 +264,21 @@ pub fn dummy_overrides() -> WasmOverride {
#[cfg(test)]
mod tests {
use super::*;
use sc_executor::{HeapAllocStrategy, NativeElseWasmExecutor, WasmExecutor};
use sc_executor::{HeapAllocStrategy, WasmExecutor};
use std::fs::{self, File};
use substrate_test_runtime_client::LocalExecutorDispatch;

fn executor() -> NativeElseWasmExecutor<substrate_test_runtime_client::LocalExecutorDispatch> {
NativeElseWasmExecutor::<substrate_test_runtime_client::LocalExecutorDispatch>::new_with_wasm_executor(
WasmExecutor::builder()
.with_onchain_heap_alloc_strategy(HeapAllocStrategy::Static {extra_pages: 128})
.with_offchain_heap_alloc_strategy(HeapAllocStrategy::Static {extra_pages: 128})
.with_max_runtime_instances(1)
.with_runtime_cache_size(2)
.build()
)

fn executor() -> WasmExecutor {
WasmExecutor::builder()
.with_onchain_heap_alloc_strategy(HeapAllocStrategy::Static { extra_pages: 128 })
.with_offchain_heap_alloc_strategy(HeapAllocStrategy::Static { extra_pages: 128 })
.with_max_runtime_instances(1)
.with_runtime_cache_size(2)
.build()
}

fn wasm_test<F>(fun: F)
where
F: Fn(&Path, &[u8], &NativeElseWasmExecutor<LocalExecutorDispatch>),
F: Fn(&Path, &[u8], &WasmExecutor),
{
let exec = executor();
let bytes = substrate_test_runtime::wasm_binary_unwrap();
Expand Down
9 changes: 5 additions & 4 deletions substrate/client/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,15 @@ use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
pub use self::{
builder::{
build_network, new_client, new_db_backend, new_full_client, new_full_parts,
new_full_parts_record_import, new_full_parts_with_genesis_builder,
new_native_or_wasm_executor, new_wasm_executor, spawn_tasks, BuildNetworkParams,
KeystoreContainer, NetworkStarter, SpawnTasksParams, TFullBackend, TFullCallExecutor,
TFullClient,
new_full_parts_record_import, new_full_parts_with_genesis_builder, new_wasm_executor,
spawn_tasks, BuildNetworkParams, KeystoreContainer, NetworkStarter, SpawnTasksParams,
TFullBackend, TFullCallExecutor, TFullClient,
},
client::{ClientConfig, LocalCallExecutor},
error::Error,
};
#[allow(deprecated)]
pub use builder::new_native_or_wasm_executor;

pub use sc_chain_spec::{
construct_genesis_block, resolve_state_version_from_wasm, BuildGenesisBlock,
Expand Down
Loading
Loading