From d1bfd9086df2571a6998971288ca689c4a802bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 28 May 2020 23:24:27 +0100 Subject: [PATCH 1/9] client: use appropriate ExecutionContext for sync/import --- client/service/src/client/client.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index 77b3f065f43dd..6c948f72df7d5 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -28,8 +28,9 @@ use parking_lot::{Mutex, RwLock}; use codec::{Encode, Decode}; use hash_db::Prefix; use sp_core::{ - ChangesTrieConfiguration, convert_hash, NativeOrEncoded, - storage::{StorageKey, PrefixedStorageKey, StorageData, well_known_keys, ChildInfo}, + convert_hash, + storage::{well_known_keys, ChildInfo, PrefixedStorageKey, StorageData, StorageKey}, + ChangesTrieConfiguration, ExecutionContext, NativeOrEncoded, }; use sc_telemetry::{telemetry, SUBSTRATE_INFO}; use sp_runtime::{ @@ -865,9 +866,15 @@ impl Client where // block. (true, ref mut storage_changes @ None, Some(ref body)) => { let runtime_api = self.runtime_api(); + let execution_context = if import_block.origin == BlockOrigin::NetworkInitialSync { + ExecutionContext::Syncing + } else { + ExecutionContext::Importing + }; - runtime_api.execute_block( + runtime_api.execute_block_with_context( &at, + execution_context, Block::new(import_block.header.clone(), body.clone()), )?; From 06315f59ded592f2014991b62d6e14251229315b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 28 May 2020 23:28:25 +0100 Subject: [PATCH 2/9] client: remove dead code --- client/service/src/client/client.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index 6c948f72df7d5..aaa09e66eb881 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -755,11 +755,6 @@ impl Client where ) = storage_changes.into_inner(); if self.config.offchain_indexing_api { - // if let Some(mut offchain_storage) = self.backend.offchain_storage() { - // offchain_sc.iter().for_each(|(k,v)| { - // offchain_storage.set(b"block-import-info", k,v) - // }); - // } operation.op.update_offchain_storage(offchain_sc)?; } From 0fac11520704c364a82432c5b927e987ba043cdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 29 May 2020 00:18:45 +0100 Subject: [PATCH 3/9] client: ExecutionContext: distinguish between own and foreign imports --- bin/node/testing/src/bench.rs | 10 ++++---- client/api/src/execution_extensions.rs | 20 ++++++++-------- client/cli/src/arg_enums.rs | 9 ++++---- client/cli/src/params/import_params.rs | 32 ++++++++++++++------------ client/service/src/client/client.rs | 6 ++--- primitives/core/src/lib.rs | 10 ++++---- test-utils/client/src/lib.rs | 9 +++----- 7 files changed, 48 insertions(+), 48 deletions(-) diff --git a/bin/node/testing/src/bench.rs b/bin/node/testing/src/bench.rs index 5b8f2e2083858..3e71534fa17ee 100644 --- a/bin/node/testing/src/bench.rs +++ b/bin/node/testing/src/bench.rs @@ -515,19 +515,19 @@ impl Profile { fn into_execution_strategies(self) -> ExecutionStrategies { match self { Profile::Wasm => ExecutionStrategies { - syncing: ExecutionStrategy::AlwaysWasm, - importing: ExecutionStrategy::AlwaysWasm, + own_block_import: ExecutionStrategy::AlwaysWasm, + foreign_block_import: ExecutionStrategy::AlwaysWasm, block_construction: ExecutionStrategy::AlwaysWasm, offchain_worker: ExecutionStrategy::AlwaysWasm, other: ExecutionStrategy::AlwaysWasm, }, Profile::Native => ExecutionStrategies { - syncing: ExecutionStrategy::NativeElseWasm, - importing: ExecutionStrategy::NativeElseWasm, + own_block_import: ExecutionStrategy::NativeElseWasm, + foreign_block_import: ExecutionStrategy::NativeElseWasm, block_construction: ExecutionStrategy::NativeElseWasm, offchain_worker: ExecutionStrategy::NativeElseWasm, other: ExecutionStrategy::NativeElseWasm, - } + }, } } } diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs index 55ffc3794c4ea..22bbe01d0ef9d 100644 --- a/client/api/src/execution_extensions.rs +++ b/client/api/src/execution_extensions.rs @@ -38,10 +38,10 @@ use parking_lot::RwLock; /// Execution strategies settings. #[derive(Debug, Clone)] pub struct ExecutionStrategies { - /// Execution strategy used when syncing. - pub syncing: ExecutionStrategy, - /// Execution strategy used when importing blocks. - pub importing: ExecutionStrategy, + /// Execution strategy used when importing locally authored blocks. + pub own_block_import: ExecutionStrategy, + /// Execution strategy used when importing foreign blocks. + pub foreign_block_import: ExecutionStrategy, /// Execution strategy used when constructing blocks. pub block_construction: ExecutionStrategy, /// Execution strategy used for offchain workers. @@ -53,8 +53,8 @@ pub struct ExecutionStrategies { impl Default for ExecutionStrategies { fn default() -> ExecutionStrategies { ExecutionStrategies { - syncing: ExecutionStrategy::NativeElseWasm, - importing: ExecutionStrategy::NativeElseWasm, + own_block_import: ExecutionStrategy::NativeElseWasm, + foreign_block_import: ExecutionStrategy::NativeElseWasm, block_construction: ExecutionStrategy::AlwaysWasm, offchain_worker: ExecutionStrategy::NativeWhenPossible, other: ExecutionStrategy::NativeElseWasm, @@ -145,10 +145,10 @@ impl ExecutionExtensions { let manager = match context { ExecutionContext::BlockConstruction => self.strategies.block_construction.get_manager(), - ExecutionContext::Syncing => - self.strategies.syncing.get_manager(), - ExecutionContext::Importing => - self.strategies.importing.get_manager(), + ExecutionContext::OwnBlockImport => + self.strategies.own_block_import.get_manager(), + ExecutionContext::ForeignBlockImport => + self.strategies.foreign_block_import.get_manager(), ExecutionContext::OffchainCall(Some((_, capabilities))) if capabilities.has_all() => self.strategies.offchain_worker.get_manager(), ExecutionContext::OffchainCall(_) => diff --git a/client/cli/src/arg_enums.rs b/client/cli/src/arg_enums.rs index 4dfd384d9513c..f1f75ab3c7f46 100644 --- a/client/cli/src/arg_enums.rs +++ b/client/cli/src/arg_enums.rs @@ -174,10 +174,11 @@ arg_enum! { } } -/// Default value for the `--execution-syncing` parameter. -pub const DEFAULT_EXECUTION_SYNCING: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; -/// Default value for the `--execution-import-block` parameter. -pub const DEFAULT_EXECUTION_IMPORT_BLOCK: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-own-block-import` parameter. +pub const DEFAULT_EXECUTION_OWN_BLOCK_IMPORT: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-foreign-block-import` parameter. +pub const DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT: ExecutionStrategy = + ExecutionStrategy::NativeElseWasm; /// Default value for the `--execution-block-construction` parameter. pub const DEFAULT_EXECUTION_BLOCK_CONSTRUCTION: ExecutionStrategy = ExecutionStrategy::Wasm; /// Default value for the `--execution-offchain-worker` parameter. diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index fb683df6d3be9..f484e5bc088e4 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -17,9 +17,9 @@ // along with this program. If not, see . use crate::arg_enums::{ - ExecutionStrategy, TracingReceiver, WasmExecutionMethod, - DEFAULT_EXECUTION_BLOCK_CONSTRUCTION, DEFAULT_EXECUTION_IMPORT_BLOCK, - DEFAULT_EXECUTION_OFFCHAIN_WORKER, DEFAULT_EXECUTION_OTHER, DEFAULT_EXECUTION_SYNCING, + ExecutionStrategy, TracingReceiver, WasmExecutionMethod, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION, + DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT, DEFAULT_EXECUTION_OFFCHAIN_WORKER, + DEFAULT_EXECUTION_OTHER, DEFAULT_EXECUTION_OWN_BLOCK_IMPORT, }; use crate::params::DatabaseParams; use crate::params::PruningParams; @@ -118,8 +118,10 @@ impl ImportParams { }; ExecutionStrategies { - syncing: exec_all_or(exec.execution_syncing, DEFAULT_EXECUTION_SYNCING), - importing: exec_all_or(exec.execution_import_block, DEFAULT_EXECUTION_IMPORT_BLOCK), + own_block_import: + exec_all_or(exec.execution_own_block_import, DEFAULT_EXECUTION_OWN_BLOCK_IMPORT), + foreign_block_import: + exec_all_or(exec.execution_foreign_block_import, DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT), block_construction: exec_all_or(exec.execution_block_construction, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION), offchain_worker: @@ -132,25 +134,25 @@ impl ImportParams { /// Execution strategies parameters. #[derive(Debug, StructOpt, Clone)] pub struct ExecutionStrategiesParams { - /// The means of execution used when calling into the runtime while syncing blocks. + /// The means of execution used when calling into the runtime while import locally authored blocks. #[structopt( - long = "execution-syncing", + long = "execution-own-block-import", value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_SYNCING.as_str(), + default_value = DEFAULT_EXECUTION_OWN_BLOCK_IMPORT.as_str(), )] - pub execution_syncing: ExecutionStrategy, + pub execution_own_block_import: ExecutionStrategy, - /// The means of execution used when calling into the runtime while importing blocks. + /// The means of execution used when calling into the runtime while importing foreign blocks. #[structopt( - long = "execution-import-block", + long = "execution-foreign-block-import", value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_IMPORT_BLOCK.as_str(), + default_value = DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT.as_str(), )] - pub execution_import_block: ExecutionStrategy, + pub execution_foreign_block_import: ExecutionStrategy, /// The means of execution used when calling into the runtime while constructing blocks. #[structopt( @@ -192,8 +194,8 @@ pub struct ExecutionStrategiesParams { "execution-other", "execution-offchain-worker", "execution-block-construction", - "execution-import-block", - "execution-syncing", + "execution-own-block-import", + "execution-foreign-block-import", ] )] pub execution: Option, diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index aaa09e66eb881..ca0bcb0e00eba 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -861,10 +861,10 @@ impl Client where // block. (true, ref mut storage_changes @ None, Some(ref body)) => { let runtime_api = self.runtime_api(); - let execution_context = if import_block.origin == BlockOrigin::NetworkInitialSync { - ExecutionContext::Syncing + let execution_context = if import_block.origin == BlockOrigin::Own { + ExecutionContext::OwnBlockImport } else { - ExecutionContext::Importing + ExecutionContext::ForeignBlockImport }; runtime_api.execute_block_with_context( diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index 56dbbc7b7898d..e6baaf6075def 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -93,10 +93,10 @@ pub use sp_std; /// Context for executing a call into the runtime. pub enum ExecutionContext { - /// Context for general importing (including own blocks). - Importing, - /// Context used when syncing the blockchain. - Syncing, + /// Context used for importing locally authored blocks. + OwnBlockImport, + /// Context used for importing foreign blocks. + ForeignBlockImport, /// Context used for block construction. BlockConstruction, /// Context used for offchain calls. @@ -111,7 +111,7 @@ impl ExecutionContext { use ExecutionContext::*; match self { - Importing | Syncing | BlockConstruction => + OwnBlockImport | ForeignBlockImport | BlockConstruction => offchain::Capabilities::none(), // Enable keystore and transaction pool by default for offchain calls. OffchainCall(None) => [ diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index ffd93970f41da..b9ec20fd25d30 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -147,13 +147,10 @@ impl TestClientBuilder Self { + pub fn set_execution_strategy(mut self, execution_strategy: ExecutionStrategy) -> Self { self.execution_strategies = ExecutionStrategies { - syncing: execution_strategy, - importing: execution_strategy, + own_block_import: execution_strategy, + foreign_block_import: execution_strategy, block_construction: execution_strategy, offchain_worker: execution_strategy, other: execution_strategy, From 1c9a9bdef5767bed9bcdc78b1e89cfc9afd0478d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 29 May 2020 00:22:02 +0100 Subject: [PATCH 4/9] client: fix cli parameter doc --- client/cli/src/params/import_params.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index f484e5bc088e4..047c6a15ece36 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -134,7 +134,7 @@ impl ImportParams { /// Execution strategies parameters. #[derive(Debug, StructOpt, Clone)] pub struct ExecutionStrategiesParams { - /// The means of execution used when calling into the runtime while import locally authored blocks. + /// The means of execution used when calling into the runtime while importing locally authored blocks. #[structopt( long = "execution-own-block-import", value_name = "STRATEGY", From f6cf8c96c82a97fb9ba95b564fc1f871306c5844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 29 May 2020 12:41:00 +0100 Subject: [PATCH 5/9] Revert "client: ExecutionContext: distinguish between own and foreign imports" This reverts commit 0fac11520704c364a82432c5b927e987ba043cdb. --- bin/node/testing/src/bench.rs | 10 ++++---- client/api/src/execution_extensions.rs | 20 +++++++-------- client/cli/src/arg_enums.rs | 9 +++---- client/cli/src/params/import_params.rs | 34 +++++++++++++------------- client/service/src/client/client.rs | 6 ++--- primitives/core/src/lib.rs | 10 ++++---- test-utils/client/src/lib.rs | 9 ++++--- 7 files changed, 50 insertions(+), 48 deletions(-) diff --git a/bin/node/testing/src/bench.rs b/bin/node/testing/src/bench.rs index 3e71534fa17ee..5b8f2e2083858 100644 --- a/bin/node/testing/src/bench.rs +++ b/bin/node/testing/src/bench.rs @@ -515,19 +515,19 @@ impl Profile { fn into_execution_strategies(self) -> ExecutionStrategies { match self { Profile::Wasm => ExecutionStrategies { - own_block_import: ExecutionStrategy::AlwaysWasm, - foreign_block_import: ExecutionStrategy::AlwaysWasm, + syncing: ExecutionStrategy::AlwaysWasm, + importing: ExecutionStrategy::AlwaysWasm, block_construction: ExecutionStrategy::AlwaysWasm, offchain_worker: ExecutionStrategy::AlwaysWasm, other: ExecutionStrategy::AlwaysWasm, }, Profile::Native => ExecutionStrategies { - own_block_import: ExecutionStrategy::NativeElseWasm, - foreign_block_import: ExecutionStrategy::NativeElseWasm, + syncing: ExecutionStrategy::NativeElseWasm, + importing: ExecutionStrategy::NativeElseWasm, block_construction: ExecutionStrategy::NativeElseWasm, offchain_worker: ExecutionStrategy::NativeElseWasm, other: ExecutionStrategy::NativeElseWasm, - }, + } } } } diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs index 22bbe01d0ef9d..55ffc3794c4ea 100644 --- a/client/api/src/execution_extensions.rs +++ b/client/api/src/execution_extensions.rs @@ -38,10 +38,10 @@ use parking_lot::RwLock; /// Execution strategies settings. #[derive(Debug, Clone)] pub struct ExecutionStrategies { - /// Execution strategy used when importing locally authored blocks. - pub own_block_import: ExecutionStrategy, - /// Execution strategy used when importing foreign blocks. - pub foreign_block_import: ExecutionStrategy, + /// Execution strategy used when syncing. + pub syncing: ExecutionStrategy, + /// Execution strategy used when importing blocks. + pub importing: ExecutionStrategy, /// Execution strategy used when constructing blocks. pub block_construction: ExecutionStrategy, /// Execution strategy used for offchain workers. @@ -53,8 +53,8 @@ pub struct ExecutionStrategies { impl Default for ExecutionStrategies { fn default() -> ExecutionStrategies { ExecutionStrategies { - own_block_import: ExecutionStrategy::NativeElseWasm, - foreign_block_import: ExecutionStrategy::NativeElseWasm, + syncing: ExecutionStrategy::NativeElseWasm, + importing: ExecutionStrategy::NativeElseWasm, block_construction: ExecutionStrategy::AlwaysWasm, offchain_worker: ExecutionStrategy::NativeWhenPossible, other: ExecutionStrategy::NativeElseWasm, @@ -145,10 +145,10 @@ impl ExecutionExtensions { let manager = match context { ExecutionContext::BlockConstruction => self.strategies.block_construction.get_manager(), - ExecutionContext::OwnBlockImport => - self.strategies.own_block_import.get_manager(), - ExecutionContext::ForeignBlockImport => - self.strategies.foreign_block_import.get_manager(), + ExecutionContext::Syncing => + self.strategies.syncing.get_manager(), + ExecutionContext::Importing => + self.strategies.importing.get_manager(), ExecutionContext::OffchainCall(Some((_, capabilities))) if capabilities.has_all() => self.strategies.offchain_worker.get_manager(), ExecutionContext::OffchainCall(_) => diff --git a/client/cli/src/arg_enums.rs b/client/cli/src/arg_enums.rs index f1f75ab3c7f46..4dfd384d9513c 100644 --- a/client/cli/src/arg_enums.rs +++ b/client/cli/src/arg_enums.rs @@ -174,11 +174,10 @@ arg_enum! { } } -/// Default value for the `--execution-own-block-import` parameter. -pub const DEFAULT_EXECUTION_OWN_BLOCK_IMPORT: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; -/// Default value for the `--execution-foreign-block-import` parameter. -pub const DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT: ExecutionStrategy = - ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-syncing` parameter. +pub const DEFAULT_EXECUTION_SYNCING: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-import-block` parameter. +pub const DEFAULT_EXECUTION_IMPORT_BLOCK: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; /// Default value for the `--execution-block-construction` parameter. pub const DEFAULT_EXECUTION_BLOCK_CONSTRUCTION: ExecutionStrategy = ExecutionStrategy::Wasm; /// Default value for the `--execution-offchain-worker` parameter. diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index 047c6a15ece36..f98652e9fddb7 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -17,9 +17,9 @@ // along with this program. If not, see . use crate::arg_enums::{ - ExecutionStrategy, TracingReceiver, WasmExecutionMethod, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION, - DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT, DEFAULT_EXECUTION_OFFCHAIN_WORKER, - DEFAULT_EXECUTION_OTHER, DEFAULT_EXECUTION_OWN_BLOCK_IMPORT, + ExecutionStrategy, TracingReceiver, WasmExecutionMethod, + DEFAULT_EXECUTION_BLOCK_CONSTRUCTION, DEFAULT_EXECUTION_IMPORT_BLOCK, + DEFAULT_EXECUTION_OFFCHAIN_WORKER, DEFAULT_EXECUTION_OTHER, DEFAULT_EXECUTION_SYNCING, }; use crate::params::DatabaseParams; use crate::params::PruningParams; @@ -118,10 +118,8 @@ impl ImportParams { }; ExecutionStrategies { - own_block_import: - exec_all_or(exec.execution_own_block_import, DEFAULT_EXECUTION_OWN_BLOCK_IMPORT), - foreign_block_import: - exec_all_or(exec.execution_foreign_block_import, DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT), + syncing: exec_all_or(exec.execution_syncing, DEFAULT_EXECUTION_SYNCING), + importing: exec_all_or(exec.execution_import_block, DEFAULT_EXECUTION_IMPORT_BLOCK), block_construction: exec_all_or(exec.execution_block_construction, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION), offchain_worker: @@ -134,25 +132,27 @@ impl ImportParams { /// Execution strategies parameters. #[derive(Debug, StructOpt, Clone)] pub struct ExecutionStrategiesParams { - /// The means of execution used when calling into the runtime while importing locally authored blocks. + /// The means of execution used when calling into the runtime while performing an + /// initial sync. #[structopt( - long = "execution-own-block-import", + long = "execution-syncing", value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_OWN_BLOCK_IMPORT.as_str(), + default_value = DEFAULT_EXECUTION_SYNCING.as_str(), )] - pub execution_own_block_import: ExecutionStrategy, + pub execution_syncing: ExecutionStrategy, - /// The means of execution used when calling into the runtime while importing foreign blocks. + /// The means of execution used when calling into the runtime while importing blocks + /// (including locally authored blocks). #[structopt( - long = "execution-foreign-block-import", + long = "execution-import-block", value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_FOREIGN_BLOCK_IMPORT.as_str(), + default_value = DEFAULT_EXECUTION_IMPORT_BLOCK.as_str(), )] - pub execution_foreign_block_import: ExecutionStrategy, + pub execution_import_block: ExecutionStrategy, /// The means of execution used when calling into the runtime while constructing blocks. #[structopt( @@ -194,8 +194,8 @@ pub struct ExecutionStrategiesParams { "execution-other", "execution-offchain-worker", "execution-block-construction", - "execution-own-block-import", - "execution-foreign-block-import", + "execution-import-block", + "execution-syncing", ] )] pub execution: Option, diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index ca0bcb0e00eba..aaa09e66eb881 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -861,10 +861,10 @@ impl Client where // block. (true, ref mut storage_changes @ None, Some(ref body)) => { let runtime_api = self.runtime_api(); - let execution_context = if import_block.origin == BlockOrigin::Own { - ExecutionContext::OwnBlockImport + let execution_context = if import_block.origin == BlockOrigin::NetworkInitialSync { + ExecutionContext::Syncing } else { - ExecutionContext::ForeignBlockImport + ExecutionContext::Importing }; runtime_api.execute_block_with_context( diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index e6baaf6075def..56dbbc7b7898d 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -93,10 +93,10 @@ pub use sp_std; /// Context for executing a call into the runtime. pub enum ExecutionContext { - /// Context used for importing locally authored blocks. - OwnBlockImport, - /// Context used for importing foreign blocks. - ForeignBlockImport, + /// Context for general importing (including own blocks). + Importing, + /// Context used when syncing the blockchain. + Syncing, /// Context used for block construction. BlockConstruction, /// Context used for offchain calls. @@ -111,7 +111,7 @@ impl ExecutionContext { use ExecutionContext::*; match self { - OwnBlockImport | ForeignBlockImport | BlockConstruction => + Importing | Syncing | BlockConstruction => offchain::Capabilities::none(), // Enable keystore and transaction pool by default for offchain calls. OffchainCall(None) => [ diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index b9ec20fd25d30..ffd93970f41da 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -147,10 +147,13 @@ impl TestClientBuilder Self { + pub fn set_execution_strategy( + mut self, + execution_strategy: ExecutionStrategy + ) -> Self { self.execution_strategies = ExecutionStrategies { - own_block_import: execution_strategy, - foreign_block_import: execution_strategy, + syncing: execution_strategy, + importing: execution_strategy, block_construction: execution_strategy, offchain_worker: execution_strategy, other: execution_strategy, From 816ea63e7cdeda75e46cf2cef28d419d6ba39c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 29 May 2020 12:45:08 +0100 Subject: [PATCH 6/9] primitives: add docs for ExecutionContext --- primitives/core/src/lib.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index 56dbbc7b7898d..5fbbf3ca6d5ee 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -93,9 +93,16 @@ pub use sp_std; /// Context for executing a call into the runtime. pub enum ExecutionContext { - /// Context for general importing (including own blocks). + /// Context used for general block import (including locally authored blocks). Importing, - /// Context used when syncing the blockchain. + /// Context used for importing blocks as part of an initial sync of the blockchain. + /// + /// We distinguish between major sync and import so that validators who are running + /// their initial sync (or catching up after some time offline) can use the faster + /// native runtime (since we can reasonably assume the network as a whole has already + /// come to a broad conensus on the block and it probably hasn't been crafted + /// specifically to attack this node), but when importing blocks at the head of the + /// chain in normal operation they can use the safer Wasm version. Syncing, /// Context used for block construction. BlockConstruction, From e6340b9a978cd112f0dfc839584938726d86ae44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 29 May 2020 13:06:57 +0100 Subject: [PATCH 7/9] cli: execution strategy docs --- client/cli/src/params/import_params.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index f98652e9fddb7..cfd5786c1985c 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -132,8 +132,8 @@ impl ImportParams { /// Execution strategies parameters. #[derive(Debug, StructOpt, Clone)] pub struct ExecutionStrategiesParams { - /// The means of execution used when calling into the runtime while performing an - /// initial sync. + /// The means of execution used when calling into the runtime for importing blocks as + /// part of an initial sync. #[structopt( long = "execution-syncing", value_name = "STRATEGY", @@ -143,7 +143,7 @@ pub struct ExecutionStrategiesParams { )] pub execution_syncing: ExecutionStrategy, - /// The means of execution used when calling into the runtime while importing blocks + /// The means of execution used when calling into the runtime for general block import /// (including locally authored blocks). #[structopt( long = "execution-import-block", From 2715b3ee50e6d0a36b728a7c1b1b48354746ad57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 4 Jun 2020 10:12:04 +0100 Subject: [PATCH 8/9] cli: use different execution context for importing block on validator --- client/cli/src/arg_enums.rs | 2 ++ client/cli/src/commands/mod.rs | 5 ++--- client/cli/src/config.rs | 14 ++++++++++---- client/cli/src/params/import_params.rs | 17 ++++++++++------- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/client/cli/src/arg_enums.rs b/client/cli/src/arg_enums.rs index 4dfd384d9513c..db13fff76148e 100644 --- a/client/cli/src/arg_enums.rs +++ b/client/cli/src/arg_enums.rs @@ -178,6 +178,8 @@ arg_enum! { pub const DEFAULT_EXECUTION_SYNCING: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; /// Default value for the `--execution-import-block` parameter. pub const DEFAULT_EXECUTION_IMPORT_BLOCK: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-import-block` parameter when the node is a validator. +pub const DEFAULT_EXECUTION_IMPORT_BLOCK_VALIDATOR: ExecutionStrategy = ExecutionStrategy::Wasm; /// Default value for the `--execution-block-construction` parameter. pub const DEFAULT_EXECUTION_BLOCK_CONSTRUCTION: ExecutionStrategy = ExecutionStrategy::Wasm; /// Default value for the `--execution-offchain-worker` parameter. diff --git a/client/cli/src/commands/mod.rs b/client/cli/src/commands/mod.rs index 62757890ef01d..dececb2a90245 100644 --- a/client/cli/src/commands/mod.rs +++ b/client/cli/src/commands/mod.rs @@ -278,10 +278,10 @@ macro_rules! substrate_cli_subcommands { } } - fn execution_strategies(&self, is_dev: bool) + fn execution_strategies(&self, is_dev: bool, is_validator: bool) -> $crate::Result<::sc_client_api::execution_extensions::ExecutionStrategies> { match self { - $($enum::$variant(cmd) => cmd.execution_strategies(is_dev)),* + $($enum::$variant(cmd) => cmd.execution_strategies(is_dev, is_validator)),* } } @@ -409,4 +409,3 @@ macro_rules! substrate_cli_subcommands { substrate_cli_subcommands!( Subcommand => BuildSpec, ExportBlocks, ImportBlocks, CheckBlock, Revert, PurgeChain, ExportState ); - diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index a1ee1b0cc1da9..90e5f98e3d871 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -246,9 +246,14 @@ pub trait CliConfiguration: Sized { /// /// By default this is retrieved from `ImportParams` if it is available. Otherwise its /// `ExecutionStrategies::default()`. - fn execution_strategies(&self, is_dev: bool) -> Result { - Ok(self.import_params() - .map(|x| x.execution_strategies(is_dev)) + fn execution_strategies( + &self, + is_dev: bool, + is_validator: bool, + ) -> Result { + Ok(self + .import_params() + .map(|x| x.execution_strategies(is_dev, is_validator)) .unwrap_or(Default::default())) } @@ -419,6 +424,7 @@ pub trait CliConfiguration: Sized { let node_key = self.node_key(&net_config_dir)?; let role = self.role(is_dev)?; let max_runtime_instances = self.max_runtime_instances()?.unwrap_or(8); + let is_validator = role.is_network_authority(); let unsafe_pruning = self .import_params() @@ -444,7 +450,7 @@ pub trait CliConfiguration: Sized { state_cache_child_ratio: self.state_cache_child_ratio()?, pruning: self.pruning(unsafe_pruning, &role)?, wasm_method: self.wasm_method()?, - execution_strategies: self.execution_strategies(is_dev)?, + execution_strategies: self.execution_strategies(is_dev, is_validator)?, rpc_http: self.rpc_http()?, rpc_ws: self.rpc_ws()?, rpc_methods: self.rpc_methods()?, diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index cfd5786c1985c..81ef83352b42e 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -17,8 +17,8 @@ // along with this program. If not, see . use crate::arg_enums::{ - ExecutionStrategy, TracingReceiver, WasmExecutionMethod, - DEFAULT_EXECUTION_BLOCK_CONSTRUCTION, DEFAULT_EXECUTION_IMPORT_BLOCK, + ExecutionStrategy, TracingReceiver, WasmExecutionMethod, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION, + DEFAULT_EXECUTION_IMPORT_BLOCK, DEFAULT_EXECUTION_IMPORT_BLOCK_VALIDATOR, DEFAULT_EXECUTION_OFFCHAIN_WORKER, DEFAULT_EXECUTION_OTHER, DEFAULT_EXECUTION_SYNCING, }; use crate::params::DatabaseParams; @@ -104,10 +104,7 @@ impl ImportParams { } /// Get execution strategies for the parameters - pub fn execution_strategies( - &self, - is_dev: bool, - ) -> ExecutionStrategies { + pub fn execution_strategies(&self, is_dev: bool, is_validator: bool) -> ExecutionStrategies { let exec = &self.execution_strategies; let exec_all_or = |strat: ExecutionStrategy, default: ExecutionStrategy| { exec.execution.unwrap_or(if strat == default && is_dev { @@ -117,9 +114,15 @@ impl ImportParams { }).into() }; + let default_execution_import_block = if is_validator { + DEFAULT_EXECUTION_IMPORT_BLOCK_VALIDATOR + } else { + DEFAULT_EXECUTION_IMPORT_BLOCK + }; + ExecutionStrategies { syncing: exec_all_or(exec.execution_syncing, DEFAULT_EXECUTION_SYNCING), - importing: exec_all_or(exec.execution_import_block, DEFAULT_EXECUTION_IMPORT_BLOCK), + importing: exec_all_or(exec.execution_import_block, default_execution_import_block), block_construction: exec_all_or(exec.execution_block_construction, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION), offchain_worker: From 999bd84638f9fb3923ac586522f3f9ac5ef3920e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 4 Jun 2020 10:13:00 +0100 Subject: [PATCH 9/9] cli: remove defaults from execution context flags --- client/cli/src/params/import_params.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index 81ef83352b42e..60bd6e6643443 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -106,12 +106,14 @@ impl ImportParams { /// Get execution strategies for the parameters pub fn execution_strategies(&self, is_dev: bool, is_validator: bool) -> ExecutionStrategies { let exec = &self.execution_strategies; - let exec_all_or = |strat: ExecutionStrategy, default: ExecutionStrategy| { - exec.execution.unwrap_or(if strat == default && is_dev { + let exec_all_or = |strat: Option, default: ExecutionStrategy| { + let default = if is_dev { ExecutionStrategy::Native } else { - strat - }).into() + default + }; + + exec.execution.unwrap_or(strat.unwrap_or(default)).into() }; let default_execution_import_block = if is_validator { @@ -142,9 +144,8 @@ pub struct ExecutionStrategiesParams { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_SYNCING.as_str(), )] - pub execution_syncing: ExecutionStrategy, + pub execution_syncing: Option, /// The means of execution used when calling into the runtime for general block import /// (including locally authored blocks). @@ -153,9 +154,8 @@ pub struct ExecutionStrategiesParams { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_IMPORT_BLOCK.as_str(), )] - pub execution_import_block: ExecutionStrategy, + pub execution_import_block: Option, /// The means of execution used when calling into the runtime while constructing blocks. #[structopt( @@ -163,9 +163,8 @@ pub struct ExecutionStrategiesParams { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_BLOCK_CONSTRUCTION.as_str(), )] - pub execution_block_construction: ExecutionStrategy, + pub execution_block_construction: Option, /// The means of execution used when calling into the runtime while using an off-chain worker. #[structopt( @@ -173,9 +172,8 @@ pub struct ExecutionStrategiesParams { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_OFFCHAIN_WORKER.as_str(), )] - pub execution_offchain_worker: ExecutionStrategy, + pub execution_offchain_worker: Option, /// The means of execution used when calling into the runtime while not syncing, importing or constructing blocks. #[structopt( @@ -183,9 +181,8 @@ pub struct ExecutionStrategiesParams { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = DEFAULT_EXECUTION_OTHER.as_str(), )] - pub execution_other: ExecutionStrategy, + pub execution_other: Option, /// The execution strategy that should be used by all execution contexts. #[structopt(