Skip to content

Commit

Permalink
Revert "Improve CodeExecutor (paritytech#2358)"
Browse files Browse the repository at this point in the history
This reverts commit f2fe6a4.
  • Loading branch information
RomarQ committed Mar 20, 2024
1 parent b1a8ee8 commit 54d0501
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 56 deletions.
12 changes: 11 additions & 1 deletion substrate/bin/node/cli/benches/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,14 @@ fn construct_block<E: Externalities>(

// execute the block to get the real header.
executor
.call(ext, &runtime_code, "Core_initialize_block", &header.encode(), CallContext::Offchain)
.call(
ext,
&runtime_code,
"Core_initialize_block",
&header.encode(),
true,
CallContext::Offchain,
)
.0
.unwrap();

Expand All @@ -114,6 +121,7 @@ fn construct_block<E: Externalities>(
&runtime_code,
"BlockBuilder_apply_extrinsic",
&i.encode(),
true,
CallContext::Offchain,
)
.0
Expand All @@ -127,6 +135,7 @@ fn construct_block<E: Externalities>(
&runtime_code,
"BlockBuilder_finalize_block",
&[0u8; 0],
true,
CallContext::Offchain,
)
.0
Expand Down Expand Up @@ -191,6 +200,7 @@ fn bench_execute_block(c: &mut Criterion) {
&runtime_code,
"Core_execute_block",
&block.0,
false,
CallContext::Offchain,
)
.0
Expand Down
69 changes: 46 additions & 23 deletions substrate/bin/node/cli/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,11 @@ 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))).0;
let r =
executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
.0;
assert!(r.is_ok());
let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true)
.0
.unwrap();
let r = ApplyExtrinsicResult::decode(&mut &v[..]).unwrap();
Expand All @@ -217,9 +219,11 @@ 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))).0;
let r =
executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
.0;
assert!(r.is_ok());
let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
let v = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true)
.0
.unwrap();
let r = ApplyExtrinsicResult::decode(&mut &v[..]).unwrap();
Expand Down Expand Up @@ -252,12 +256,14 @@ 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))).0;
let r =
executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
.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())).0;
let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true).0;
assert!(r.is_ok());

t.execute_with(|| {
Expand Down Expand Up @@ -292,12 +298,14 @@ 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))).0;
let r =
executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
.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())).0;
let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), true).0;
assert!(r.is_ok());

t.execute_with(|| {
Expand Down Expand Up @@ -329,7 +337,7 @@ fn full_native_block_import_works() {
.base_extrinsic,
);

executor_call(&mut t, "Core_execute_block", &block1.0).0.unwrap();
executor_call(&mut t, "Core_execute_block", &block1.0, true).0.unwrap();

t.execute_with(|| {
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
Expand Down Expand Up @@ -404,7 +412,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).0.unwrap();
executor_call(&mut t, "Core_execute_block", &block2.0, true).0.unwrap();

t.execute_with(|| {
assert_eq!(
Expand Down Expand Up @@ -546,7 +554,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).0.unwrap();
executor_call(&mut t, "Core_execute_block", &block1.0, false).0.unwrap();

t.execute_with(|| {
assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - fees);
Expand All @@ -556,7 +564,7 @@ fn full_wasm_block_import_works() {

fees = t.execute_with(|| transfer_fee(&xt()));

executor_call(&mut t, "Core_execute_block", &block2.0).0.unwrap();
executor_call(&mut t, "Core_execute_block", &block2.0, false).0.unwrap();

t.execute_with(|| {
assert_eq!(
Expand Down Expand Up @@ -709,7 +717,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).0.unwrap();
executor_call(&mut t, "Core_execute_block", &b.0, false).0.unwrap();

t.execute_with(|| {
// Verify that the contract does exist by querying some of its storage items
Expand All @@ -724,15 +732,16 @@ 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).0;
let result =
executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0, false).0;
assert!(result.is_err()); // Err(Wasmi(Trap(Trap { kind: Host(AllocatorOutOfSpace) })))
}

#[test]
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)
executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0, true)
.0
.unwrap();
}
Expand All @@ -745,9 +754,11 @@ 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)
.0
.is_err());
assert!(
executor_call(&mut t, "Core_execute_block", &block_with_size(42, 0, 120_000).0, false,)
.0
.is_err()
);
}

#[test]
Expand All @@ -764,9 +775,15 @@ 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))).0;
let r = executor_call(
&mut t,
"Core_initialize_block",
&vec![].and(&from_block_number(1u32)),
false,
)
.0;
assert!(r.is_ok());
let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), false)
.0
.unwrap();
let r = ApplyExtrinsicResult::decode(&mut &r[..]).unwrap();
Expand Down Expand Up @@ -799,15 +816,21 @@ 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))).0;
let r = executor_call(
&mut t,
"Core_initialize_block",
&vec![].and(&from_block_number(1u32)),
false,
)
.0;
assert!(r.is_ok());
t.execute_with(|| {
assert_eq!(Balances::total_balance(&alice()), 111 * DOLLARS);
});

let fees = t.execute_with(|| transfer_fee(&xt()));

let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()))
let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt()), false)
.0
.unwrap();
ApplyExtrinsicResult::decode(&mut &r[..])
Expand Down Expand Up @@ -838,7 +861,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![])
let r = executor_call(&mut t, "GenesisBuilder_create_default_config", &vec![], false)
.0
.unwrap();
let r = Vec::<u8>::decode(&mut &r[..]).unwrap();
Expand Down
9 changes: 5 additions & 4 deletions substrate/bin/node/cli/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ pub fn executor_call(
t: &mut TestExternalities<BlakeTwo256>,
method: &str,
data: &[u8],
use_native: bool,
) -> (Result<Vec<u8>>, bool) {
let mut t = t.ext();

Expand All @@ -116,7 +117,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, CallContext::Onchain)
executor().call(&mut t, &runtime_code, method, data, use_native, CallContext::Onchain)
}

pub fn new_test_ext(code: &[u8]) -> TestExternalities<BlakeTwo256> {
Expand Down Expand Up @@ -167,12 +168,12 @@ pub fn construct_block(
};

// execute the block to get the real header.
executor_call(env, "Core_initialize_block", &header.encode()).0.unwrap();
executor_call(env, "Core_initialize_block", &header.encode(), true).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())
let r = executor_call(env, "BlockBuilder_apply_extrinsic", &extrinsic.encode(), true)
.0
.expect("application of an extrinsic failed");

Expand All @@ -185,7 +186,7 @@ pub fn construct_block(
}

let header = Header::decode(
&mut &executor_call(env, "BlockBuilder_finalize_block", &[0u8; 0]).0.unwrap()[..],
&mut &executor_call(env, "BlockBuilder_finalize_block", &[0u8; 0], true).0.unwrap()[..],
)
.unwrap();

Expand Down
14 changes: 8 additions & 6 deletions substrate/bin/node/cli/tests/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).0.unwrap();
executor_call(&mut t, "Core_execute_block", &block1.0, true).0.unwrap();

// weight multiplier is increased for next block.
t.execute_with(|| {
Expand All @@ -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).0.unwrap();
executor_call(&mut t, "Core_execute_block", &block2.0, true).0.unwrap();

// weight multiplier is increased for next block.
t.execute_with(|| {
Expand Down Expand Up @@ -151,10 +151,12 @@ 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))).0;
let r =
executor_call(&mut t, "Core_initialize_block", &vec![].and(&from_block_number(1u32)), true)
.0;

assert!(r.is_ok());
let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt.clone())).0;
let r = executor_call(&mut t, "BlockBuilder_apply_extrinsic", &vec![].and(&xt.clone()), true).0;
assert!(r.is_ok());

t.execute_with(|| {
Expand Down Expand Up @@ -245,7 +247,7 @@ fn block_weight_capacity_report() {
len / 1024 / 1024,
);

let r = executor_call(&mut t, "Core_execute_block", &block.0).0;
let r = executor_call(&mut t, "Core_execute_block", &block.0, true).0;

println!(" || Result = {:?}", r);
assert!(r.is_ok());
Expand Down Expand Up @@ -308,7 +310,7 @@ fn block_length_capacity_report() {
len / 1024 / 1024,
);

let r = executor_call(&mut t, "Core_execute_block", &block.0).0;
let r = executor_call(&mut t, "Core_execute_block", &block.0, true).0;

println!(" || Result = {:?}", r);
assert!(r.is_ok());
Expand Down
1 change: 1 addition & 0 deletions substrate/client/chain-spec/src/genesis_config_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ where
&RuntimeCode { heap_pages: None, code_fetcher: self, hash: self.code_hash.clone() },
method,
data,
false,
CallContext::Offchain,
)
.0
Expand Down
23 changes: 5 additions & 18 deletions substrate/client/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ where
runtime_code: &RuntimeCode,
method: &str,
data: &[u8],
_use_native: bool,
context: CallContext,
) -> (Result<Vec<u8>>, bool) {
tracing::trace!(
Expand Down Expand Up @@ -585,8 +586,6 @@ pub struct NativeElseWasmExecutor<D: NativeExecutionDispatch> {
/// Fallback wasm executor.
wasm:
WasmExecutor<ExtendedHostFunctions<sp_io::SubstrateHostFunctions, D::ExtendHostFunctions>>,

use_native: bool,
}

impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
Expand Down Expand Up @@ -623,7 +622,7 @@ impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
.with_runtime_cache_size(runtime_cache_size)
.build();

NativeElseWasmExecutor { native_version: D::native_version(), wasm, use_native: true }
NativeElseWasmExecutor { native_version: D::native_version(), wasm }
}

/// Create a new instance using the given [`WasmExecutor`].
Expand All @@ -632,14 +631,7 @@ impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
ExtendedHostFunctions<sp_io::SubstrateHostFunctions, D::ExtendHostFunctions>,
>,
) -> Self {
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;
Self { native_version: D::native_version(), wasm: executor }
}

/// Ignore missing function imports if set true.
Expand Down Expand Up @@ -674,10 +666,9 @@ 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,
Expand Down Expand Up @@ -741,11 +732,7 @@ 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(),
use_native: self.use_native,
}
NativeElseWasmExecutor { native_version: D::native_version(), wasm: self.wasm.clone() }
}
}

Expand Down
Loading

0 comments on commit 54d0501

Please sign in to comment.