From f2fe6a4c56ff524d9fd3a35eda8b246740af69e8 Mon Sep 17 00:00:00 2001
From: yjh <yjh465402634@gmail.com>
Date: Wed, 29 Nov 2023 21:48:32 +0800
Subject: [PATCH] Improve `CodeExecutor` (#2358)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Since `sp-state-machine` and `GenesisConfigBuilderRuntimeCaller` always
set `use_native` to be false.
We should remove this param and make `NativeElseWasmExecutor` behave
like its name.
It could make the above components use the correct execution strategy.
Maybe polkadot do not need about `NativeElseWasmExecutor` anymore. But
it is still needed by other chains and it's useful for debugging.
---------
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
---
substrate/bin/node/cli/benches/executor.rs | 12 +---
substrate/bin/node/cli/tests/basic.rs | 69 +++++++------------
substrate/bin/node/cli/tests/common.rs | 9 ++-
substrate/bin/node/cli/tests/fees.rs | 14 ++--
.../chain-spec/src/genesis_config_builder.rs | 1 -
substrate/client/executor/src/executor.rs | 23 +++++--
substrate/primitives/core/src/traits.rs | 1 -
substrate/primitives/state-machine/src/lib.rs | 5 +-
substrate/test-utils/client/src/lib.rs | 3 +-
9 files changed, 56 insertions(+), 81 deletions(-)
diff --git a/substrate/bin/node/cli/benches/executor.rs b/substrate/bin/node/cli/benches/executor.rs
index d654646904b..a326e1a79ea 100644
--- a/substrate/bin/node/cli/benches/executor.rs
+++ b/substrate/bin/node/cli/benches/executor.rs
@@ -103,14 +103,7 @@ fn construct_block<E: Externalities>(
// execute the block to get the real header.
executor
- .call(
- ext,
- &runtime_code,
- "Core_initialize_block",
- &header.encode(),
- true,
- CallContext::Offchain,
- )
+ .call(ext, &runtime_code, "Core_initialize_block", &header.encode(), CallContext::Offchain)
.0
.unwrap();
@@ -121,7 +114,6 @@ fn construct_block<E: Externalities>(
&runtime_code,
"BlockBuilder_apply_extrinsic",
&i.encode(),
- true,
CallContext::Offchain,
)
.0
@@ -135,7 +127,6 @@ fn construct_block<E: Externalities>(
&runtime_code,
"BlockBuilder_finalize_block",
&[0u8; 0],
- true,
CallContext::Offchain,
)
.0
@@ -200,7 +191,6 @@ fn bench_execute_block(c: &mut Criterion) {
&runtime_code,
"Core_execute_block",
&block.0,
- false,
CallContext::Offchain,
)
.0
diff --git a/substrate/bin/node/cli/tests/basic.rs b/substrate/bin/node/cli/tests/basic.rs
index cbceac04e8e..e5a8a397254 100644
--- a/substrate/bin/node/cli/tests/basic.rs
+++ b/substrate/bin/node/cli/tests/basic.rs
@@ -193,11 +193,9 @@ fn panic_execution_with_foreign_code_gives_error() {
t.insert(<pallet_balances::TotalIssuance<Runtime>>::hashed_key().to_vec(), 69_u128.encode());
t.insert(<frame_system::BlockHash<Runtime>>::hashed_key_for(0), vec![0u8; 32]);
- let r =
- executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
- .0;
+ let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0;
assert!(r.is_ok());
- let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true)
+ let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
.0
.unwrap();
let r = ApplyExtrinsicResult::decode(&mut &v[..]).unwrap();
@@ -219,11 +217,9 @@ fn bad_extrinsic_with_native_equivalent_code_gives_error() {
t.insert(<pallet_balances::TotalIssuance<Runtime>>::hashed_key().to_vec(), 69u128.encode());
t.insert(<frame_system::BlockHash<Runtime>>::hashed_key_for(0), vec![0u8; 32]);
- let r =
- executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
- .0;
+ let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0;
assert!(r.is_ok());
- let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true)
+ let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
.0
.unwrap();
let r = ApplyExtrinsicResult::decode(&mut &v[..]).unwrap();
@@ -256,14 +252,12 @@ fn successful_execution_with_native_equivalent_code_gives_ok() {
);
t.insert(<frame_system::BlockHash<Runtime>>::hashed_key_for(0), vec![0u8; 32]);
- let r =
- executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
- .0;
+ let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0;
assert!(r.is_ok());
let fees = t.execute_with(|| transfer_fee(&xt()));
- let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true).0;
+ let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt())).0;
assert!(r.is_ok());
t.execute_with(|| {
@@ -298,14 +292,12 @@ fn successful_execution_with_foreign_code_gives_ok() {
);
t.insert(<frame_system::BlockHash<Runtime>>::hashed_key_for(0), vec![0u8; 32]);
- let r =
- executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
- .0;
+ let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0;
assert!(r.is_ok());
let fees = t.execute_with(|| transfer_fee(&xt()));
- let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true).0;
+ let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt())).0;
assert!(r.is_ok());
t.execute_with(|| {
@@ -337,7 +329,7 @@ fn full_native_block_import_works() {
.base_extrinsic,
);
- executor_call(&mut t, "Core_execute_block", &block1.0, true).0.unwrap();
+ executor_call(&mut t, "Core_execute_block", &block1.0).0.unwrap();
t.execute_with(|| {
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
@@ -412,7 +404,7 @@ fn full_native_block_import_works() {
fees = t.execute_with(|| transfer_fee(&xt()));
let pot = t.execute_with(|| Treasury::pot());
- executor_call(&mut t, "Core_execute_block", &block2.0, true).0.unwrap();
+ executor_call(&mut t, "Core_execute_block", &block2.0).0.unwrap();
t.execute_with(|| {
assert_eq!(
@@ -554,7 +546,7 @@ fn full_wasm_block_import_works() {
let mut alice_last_known_balance: Balance = Default::default();
let mut fees = t.execute_with(|| transfer_fee(&xt()));
- executor_call(&mut t, "Core_execute_block", &block1.0, false).0.unwrap();
+ executor_call(&mut t, "Core_execute_block", &block1.0).0.unwrap();
t.execute_with(|| {
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
@@ -564,7 +556,7 @@ fn full_wasm_block_import_works() {
fees = t.execute_with(|| transfer_fee(&xt()));
- executor_call(&mut t, "Core_execute_block", &block2.0, false).0.unwrap();
+ executor_call(&mut t, "Core_execute_block", &block2.0).0.unwrap();
t.execute_with(|| {
assert_eq!(
@@ -717,7 +709,7 @@ fn deploying_wasm_contract_should_work() {
let mut t = new_test_ext(compact_code_unwrap());
- executor_call(&mut t, "Core_execute_block", &b.0, false).0.unwrap();
+ executor_call(&mut t, "Core_execute_block", &b.0).0.unwrap();
t.execute_with(|| {
// Verify that the contract does exist by querying some of its storage items
@@ -732,8 +724,7 @@ fn wasm_big_block_import_fails() {
set_heap_pages(&mut t.ext(), 4);
- let result =
- executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0, false).0;
+ let result = executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0).0;
assert!(result.is_err()); // Err(Wasmi(Trap(Trap { kind: Host(AllocatorOutOfSpace) })))
}
@@ -741,7 +732,7 @@ fn wasm_big_block_import_fails() {
fn native_big_block_import_succeeds() {
let mut t = new_test_ext(compact_code_unwrap());
- executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0, true)
+ executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0)
.0
.unwrap();
}
@@ -754,11 +745,9 @@ fn native_big_block_import_fails_on_fallback() {
// block.
set_heap_pages(&mut t.ext(), 8);
- assert!(
- executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0, false,)
- .0
- .is_err()
- );
+ assert!(executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0)
+ .0
+ .is_err());
}
#[test]
@@ -775,15 +764,9 @@ fn panic_execution_gives_error() {
t.insert(<pallet_balances::TotalIssuance<Runtime>>::hashed_key().to_vec(), 0_u128.encode());
t.insert(<frame_system::BlockHash<Runtime>>::hashed_key_for(0), vec![0u8; 32]);
- let r = executor_call(
- &mut t,
- "Core_initialize_block",
- &vec![].and(&from_block_number(1u32)),
- false,
- )
- .0;
+ let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0;
assert!(r.is_ok());
- let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), false)
+ let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
.0
.unwrap();
let r = ApplyExtrinsicResult::decode(&mut &r[..]).unwrap();
@@ -816,13 +799,7 @@ fn successful_execution_gives_ok() {
);
t.insert(<frame_system::BlockHash<Runtime>>::hashed_key_for(0), vec![0u8; 32]);
- let r = executor_call(
- &mut t,
- "Core_initialize_block",
- &vec![].and(&from_block_number(1u32)),
- false,
- )
- .0;
+ let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0;
assert!(r.is_ok());
t.execute_with(|| {
assert_eq!(Balances::total_balance(&alice()), 111 * DOLLARS);
@@ -830,7 +807,7 @@ fn successful_execution_gives_ok() {
let fees = t.execute_with(|| transfer_fee(&xt()));
- let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), false)
+ let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
.0
.unwrap();
ApplyExtrinsicResult::decode(&mut &r[..])
@@ -861,7 +838,7 @@ fn should_import_block_with_test_client() {
#[test]
fn default_config_as_json_works() {
let mut t = new_test_ext(compact_code_unwrap());
- let r = executor_call(&mut t, "GenesisBuilder_create_default_config", &vec![], false)
+ let r = executor_call(&mut t, "GenesisBuilder_create_default_config", &vec![])
.0
.unwrap();
let r = Vec::<u8>::decode(&mut &r[..]).unwrap();
diff --git a/substrate/bin/node/cli/tests/common.rs b/substrate/bin/node/cli/tests/common.rs
index 7d8f6a9a0e7..9019594ff62 100644
--- a/substrate/bin/node/cli/tests/common.rs
+++ b/substrate/bin/node/cli/tests/common.rs
@@ -105,7 +105,6 @@ pub fn executor_call(
t: &mut TestExternalities<BlakeTwo256>,
method: &str,
data: &[u8],
- use_native: bool,
) -> (Result<Vec<u8>>, bool) {
let mut t = t.ext();
@@ -117,7 +116,7 @@ pub fn executor_call(
heap_pages: heap_pages.and_then(|hp| Decode::decode(&mut &hp[..]).ok()),
};
sp_tracing::try_init_simple();
- executor().call(&mut t, &runtime_code, method, data, use_native, CallContext::Onchain)
+ executor().call(&mut t, &runtime_code, method, data, CallContext::Onchain)
}
pub fn new_test_ext(code: &[u8]) -> TestExternalities<BlakeTwo256> {
@@ -168,12 +167,12 @@ pub fn construct_block(
};
// execute the block to get the real header.
- executor_call(env, "Core_initialize_block", &header.encode(), true).0.unwrap();
+ executor_call(env, "Core_initialize_block", &header.encode()).0.unwrap();
for extrinsic in extrinsics.iter() {
// Try to apply the `extrinsic`. It should be valid, in the sense that it passes
// all pre-inclusion checks.
- let r = executor_call(env, "BlockBuilder_apply_extrinsic", &extrinsic.encode(), true)
+ let r = executor_call(env, "BlockBuilder_apply_extrinsic", &extrinsic.encode())
.0
.expect("application of an extrinsic failed");
@@ -186,7 +185,7 @@ pub fn construct_block(
}
let header = Header::decode(
- &mut &executor_call(env, "BlockBuilder_finalize_block", &[0u8; 0], true).0.unwrap()[..],
+ &mut &executor_call(env, "BlockBuilder_finalize_block", &[0u8; 0]).0.unwrap()[..],
)
.unwrap();
diff --git a/substrate/bin/node/cli/tests/fees.rs b/substrate/bin/node/cli/tests/fees.rs
index 7519ce6e8b1..8c7b3c87315 100644
--- a/substrate/bin/node/cli/tests/fees.rs
+++ b/substrate/bin/node/cli/tests/fees.rs
@@ -95,7 +95,7 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
);
// execute a big block.
- executor_call(&mut t, "Core_execute_block", &block1.0, true).0.unwrap();
+ executor_call(&mut t, "Core_execute_block", &block1.0).0.unwrap();
// weight multiplier is increased for next block.
t.execute_with(|| {
@@ -106,7 +106,7 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
});
// execute a big block.
- executor_call(&mut t, "Core_execute_block", &block2.0, true).0.unwrap();
+ executor_call(&mut t, "Core_execute_block", &block2.0).0.unwrap();
// weight multiplier is increased for next block.
t.execute_with(|| {
@@ -151,12 +151,10 @@ fn transaction_fee_is_correct() {
function: RuntimeCall::Balances(default_transfer_call()),
});
- let r =
- executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
- .0;
+ let r = executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32))).0;
assert!(r.is_ok());
- let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt.clone()), true).0;
+ let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt.clone())).0;
assert!(r.is_ok());
t.execute_with(|| {
@@ -247,7 +245,7 @@ fn block_weight_capacity_report() {
len / 1024 / 1024,
);
- let r = executor_call(&mut t, "Core_execute_block", &block.0, true).0;
+ let r = executor_call(&mut t, "Core_execute_block", &block.0).0;
println!(" || Result = {:?}", r);
assert!(r.is_ok());
@@ -310,7 +308,7 @@ fn block_length_capacity_report() {
len / 1024 / 1024,
);
- let r = executor_call(&mut t, "Core_execute_block", &block.0, true).0;
+ let r = executor_call(&mut t, "Core_execute_block", &block.0).0;
println!(" || Result = {:?}", r);
assert!(r.is_ok());
diff --git a/substrate/client/chain-spec/src/genesis_config_builder.rs b/substrate/client/chain-spec/src/genesis_config_builder.rs
index 9ccf6b4efb2..68f3d886049 100644
--- a/substrate/client/chain-spec/src/genesis_config_builder.rs
+++ b/substrate/client/chain-spec/src/genesis_config_builder.rs
@@ -76,7 +76,6 @@ where
&RuntimeCode { heap_pages: None, code_fetcher: self, hash: self.code_hash.clone() },
method,
data,
- false,
CallContext::Offchain,
)
.0
diff --git a/substrate/client/executor/src/executor.rs b/substrate/client/executor/src/executor.rs
index 7c292a83da0..499bb704b16 100644
--- a/substrate/client/executor/src/executor.rs
+++ b/substrate/client/executor/src/executor.rs
@@ -492,7 +492,6 @@ where
runtime_code: &RuntimeCode,
method: &str,
data: &[u8],
- _use_native: bool,
context: CallContext,
) -> (Result<Vec<u8>>, bool) {
tracing::trace!(
@@ -565,6 +564,8 @@ pub struct NativeElseWasmExecutor<D: NativeExecutionDispatch> {
/// Fallback wasm executor.
wasm:
WasmExecutor<ExtendedHostFunctions<sp_io::SubstrateHostFunctions, D::ExtendHostFunctions>>,
+
+ use_native: bool,
}
impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
@@ -601,7 +602,7 @@ impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
.with_runtime_cache_size(runtime_cache_size)
.build();
- NativeElseWasmExecutor { native_version: D::native_version(), wasm }
+ NativeElseWasmExecutor { native_version: D::native_version(), wasm, use_native: true }
}
/// Create a new instance using the given [`WasmExecutor`].
@@ -610,7 +611,14 @@ impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
ExtendedHostFunctions<sp_io::SubstrateHostFunctions, D::ExtendHostFunctions>,
>,
) -> Self {
- Self { native_version: D::native_version(), wasm: executor }
+ Self { native_version: D::native_version(), wasm: executor, use_native: true }
+ }
+
+ /// Disable to use native runtime when possible just behave like `WasmExecutor`.
+ ///
+ /// Default to enabled.
+ pub fn disable_use_native(&mut self) {
+ self.use_native = false;
}
/// Ignore missing function imports if set true.
@@ -645,9 +653,10 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecut
runtime_code: &RuntimeCode,
method: &str,
data: &[u8],
- use_native: bool,
context: CallContext,
) -> (Result<Vec<u8>>, bool) {
+ let use_native = self.use_native;
+
tracing::trace!(
target: "executor",
function = %method,
@@ -711,7 +720,11 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecut
impl<D: NativeExecutionDispatch> Clone for NativeElseWasmExecutor<D> {
fn clone(&self) -> Self {
- NativeElseWasmExecutor { native_version: D::native_version(), wasm: self.wasm.clone() }
+ NativeElseWasmExecutor {
+ native_version: D::native_version(),
+ wasm: self.wasm.clone(),
+ use_native: self.use_native,
+ }
}
}
diff --git a/substrate/primitives/core/src/traits.rs b/substrate/primitives/core/src/traits.rs
index 9815c84f339..851d8910391 100644
--- a/substrate/primitives/core/src/traits.rs
+++ b/substrate/primitives/core/src/traits.rs
@@ -51,7 +51,6 @@ pub trait CodeExecutor: Sized + Send + Sync + ReadRuntimeVersion + Clone + 'stat
runtime_code: &RuntimeCode,
method: &str,
data: &[u8],
- use_native: bool,
context: CallContext,
) -> (Result<Vec<u8>, Self::Error>, bool);
}
diff --git a/substrate/primitives/state-machine/src/lib.rs b/substrate/primitives/state-machine/src/lib.rs
index 0e2b9bfdfff..1345097e8f6 100644
--- a/substrate/primitives/state-machine/src/lib.rs
+++ b/substrate/primitives/state-machine/src/lib.rs
@@ -289,7 +289,7 @@ mod execution {
let result = self
.exec
- .call(&mut ext, self.runtime_code, self.method, self.call_data, false, self.context)
+ .call(&mut ext, self.runtime_code, self.method, self.call_data, self.context)
.0;
self.overlay
@@ -1120,10 +1120,9 @@ mod tests {
_: &RuntimeCode,
_method: &str,
_data: &[u8],
- use_native: bool,
_: CallContext,
) -> (CallResult<Self::Error>, bool) {
- let using_native = use_native && self.native_available;
+ let using_native = self.native_available;
match (using_native, self.native_succeeds, self.fallback_succeeds) {
(true, true, _) | (false, _, true) => (
Ok(vec![
diff --git a/substrate/test-utils/client/src/lib.rs b/substrate/test-utils/client/src/lib.rs
index 084dd2a1861..f383f7c3dc3 100644
--- a/substrate/test-utils/client/src/lib.rs
+++ b/substrate/test-utils/client/src/lib.rs
@@ -263,9 +263,10 @@ impl<Block: BlockT, D, Backend, G: GenesisInit>
D: sc_executor::NativeExecutionDispatch + 'static,
Backend: sc_client_api::backend::Backend<Block> + 'static,
{
- let executor = executor.into().unwrap_or_else(|| {
+ let mut executor = executor.into().unwrap_or_else(|| {
NativeElseWasmExecutor::new_with_wasm_executor(WasmExecutor::builder().build())
});
+ executor.disable_use_native();
let executor = LocalCallExecutor::new(
self.backend.clone(),
executor.clone(),
--
GitLab