Removal of execution strategies (#14387)

* Start

* More work!

* Moar

* More changes

* More fixes

* More worrk

* More fixes

* More fixes to make it compile

* Adds `NoOffchainStorage`

* Pass the extensions

* Small basti making small progress

* Fix merge errors and remove `ExecutionContext`

* Move registration of `ReadRuntimeVersionExt` to `ExecutionExtension`

Instead of registering `ReadRuntimeVersionExt` in `sp-state-machine` it is moved to
`ExecutionExtension` which provides the default extensions.

* Fix compilation

* Register the global extensions inside runtime api instance

* Fixes

* Fix `generate_initial_session_keys` by passing the keystore extension

* Fix the grandpa tests

* Fix more tests

* Fix more tests

* Don't set any heap pages if there isn't an override

* Fix small fallout

* FMT

* Fix tests

* More tests

* Offchain worker custom extensions

* More fixes

* Make offchain tx pool creation reusable

Introduces an `OffchainTransactionPoolFactory` for creating offchain transactions pools that can be
registered in the runtime externalities context. This factory will be required for a later pr to
make the creation of offchain transaction pools easier.

* Fixes

* Fixes

* Set offchain transaction pool in BABE before using it in the runtime

* Add the `offchain_tx_pool` to Grandpa as well

* Fix the nodes

* Print some error when using the old warnings

* Fix merge issues

* Fix compilation

* Rename `babe_link`

* Rename to `offchain_tx_pool_factory`

* Cleanup

* FMT

* Fix benchmark name

* Fix `try-runtime`

* Remove `--execution` CLI args

* Make clippy happy

* Forward bls functions

* Fix docs

* Update UI tests

* Update client/api/src/execution_extensions.rs

Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Koute <koute@users.noreply.github.com>

* Update client/cli/src/params/import_params.rs

Co-authored-by: Koute <koute@users.noreply.github.com>

* Update client/api/src/execution_extensions.rs

Co-authored-by: Koute <koute@users.noreply.github.com>

* Pass the offchain storage to the MMR RPC

* Update client/api/src/execution_extensions.rs

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>

* Review comments

* Fixes

---------

Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Co-authored-by: Koute <koute@users.noreply.github.com>
Co-authored-by: Sebastian Kunert <skunert49@gmail.com>

substrate/primitives/state-machine/src/lib.rs

@@ -168,14 +168,7 @@ mod execution {
 		traits::{CallContext, CodeExecutor, RuntimeCode},
 	};
 	use sp_externalities::Extensions;
-	use std::{
-		collections::{HashMap, HashSet},
-		fmt,
-	};
-
-	const PROOF_CLOSE_TRANSACTION: &str = "\
-		Closing a transaction that was started in this function. Client initiated transactions
-		are protected from being closed by the runtime. qed";
+	use std::collections::{HashMap, HashSet};
 
 	pub(crate) type CallResult<E> = Result<Vec<u8>, E>;
 
@@ -185,21 +178,6 @@ mod execution {
 	/// Trie backend with in-memory storage.
 	pub type InMemoryBackend<H> = TrieBackend<MemoryDB<H>, H>;
 
-	/// Strategy for executing a call into the runtime.
-	#[derive(Copy, Clone, Eq, PartialEq, Debug)]
-	pub enum ExecutionStrategy {
-		/// Execute with the native equivalent if it is compatible with the given wasm module;
-		/// otherwise fall back to the wasm.
-		NativeWhenPossible,
-		/// Use the given wasm module.
-		AlwaysWasm,
-		/// Run with both the wasm and the native variant (if compatible). Report any discrepancy
-		/// as an error.
-		Both,
-		/// First native, then if that fails or is not possible, wasm.
-		NativeElseWasm,
-	}
-
 	/// Storage backend trust level.
 	#[derive(Debug, Clone)]
 	pub enum BackendTrustLevel {
@@ -211,73 +189,6 @@ mod execution {
 		Untrusted,
 	}
 
-	/// Like `ExecutionStrategy` only it also stores a handler in case of consensus failure.
-	#[derive(Clone)]
-	pub enum ExecutionManager<F> {
-		/// Execute with the native equivalent if it is compatible with the given wasm module;
-		/// otherwise fall back to the wasm.
-		NativeWhenPossible,
-		/// Use the given wasm module. The backend on which code is executed code could be
-		/// trusted to provide all storage or not (i.e. the light client cannot be trusted to
-		/// provide for all storage queries since the storage entries it has come from an external
-		/// node).
-		AlwaysWasm(BackendTrustLevel),
-		/// Run with both the wasm and the native variant (if compatible). Call `F` in the case of
-		/// any discrepancy.
-		Both(F),
-		/// First native, then if that fails or is not possible, wasm.
-		NativeElseWasm,
-	}
-
-	impl<'a, F> From<&'a ExecutionManager<F>> for ExecutionStrategy {
-		fn from(s: &'a ExecutionManager<F>) -> Self {
-			match *s {
-				ExecutionManager::NativeWhenPossible => ExecutionStrategy::NativeWhenPossible,
-				ExecutionManager::AlwaysWasm(_) => ExecutionStrategy::AlwaysWasm,
-				ExecutionManager::NativeElseWasm => ExecutionStrategy::NativeElseWasm,
-				ExecutionManager::Both(_) => ExecutionStrategy::Both,
-			}
-		}
-	}
-
-	impl ExecutionStrategy {
-		/// Gets the corresponding manager for the execution strategy.
-		pub fn get_manager<E: fmt::Debug>(self) -> ExecutionManager<DefaultHandler<E>> {
-			match self {
-				ExecutionStrategy::AlwaysWasm =>
-					ExecutionManager::AlwaysWasm(BackendTrustLevel::Trusted),
-				ExecutionStrategy::NativeWhenPossible => ExecutionManager::NativeWhenPossible,
-				ExecutionStrategy::NativeElseWasm => ExecutionManager::NativeElseWasm,
-				ExecutionStrategy::Both => ExecutionManager::Both(|wasm_result, native_result| {
-					warn!(
-						"Consensus error between wasm {:?} and native {:?}. Using wasm.",
-						wasm_result, native_result,
-					);
-					warn!("   Native result {:?}", native_result);
-					warn!("   Wasm result {:?}", wasm_result);
-					wasm_result
-				}),
-			}
-		}
-	}
-
-	/// Evaluate to ExecutionManager::NativeElseWasm, without having to figure out the type.
-	pub fn native_else_wasm<E>() -> ExecutionManager<DefaultHandler<E>> {
-		ExecutionManager::NativeElseWasm
-	}
-
-	/// Evaluate to ExecutionManager::AlwaysWasm with trusted backend, without having to figure out
-	/// the type.
-	fn always_wasm<E>() -> ExecutionManager<DefaultHandler<E>> {
-		ExecutionManager::AlwaysWasm(BackendTrustLevel::Trusted)
-	}
-
-	/// Evaluate ExecutionManager::AlwaysWasm with untrusted backend, without having to figure out
-	/// the type.
-	fn always_untrusted_wasm<E>() -> ExecutionManager<DefaultHandler<E>> {
-		ExecutionManager::AlwaysWasm(BackendTrustLevel::Untrusted)
-	}
-
 	/// The substrate state machine.
 	pub struct StateMachine<'a, B, H, Exec>
 	where
@@ -289,7 +200,7 @@ mod execution {
 		method: &'a str,
 		call_data: &'a [u8],
 		overlay: &'a mut OverlayedChanges,
-		extensions: Extensions,
+		extensions: &'a mut Extensions,
 		storage_transaction_cache: Option<&'a mut StorageTransactionCache<B::Transaction, H>>,
 		runtime_code: &'a RuntimeCode<'a>,
 		stats: StateMachineStats,
@@ -324,7 +235,7 @@ mod execution {
 			exec: &'a Exec,
 			method: &'a str,
 			call_data: &'a [u8],
-			extensions: Extensions,
+			extensions: &'a mut Extensions,
 			runtime_code: &'a RuntimeCode,
 			context: CallContext,
 		) -> Self {
@@ -372,13 +283,7 @@ mod execution {
 		/// blocks (e.g. a transaction at a time), ensure a different method is used.
 		///
 		/// Returns the SCALE encoded result of the executed function.
-		pub fn execute(&mut self, strategy: ExecutionStrategy) -> Result<Vec<u8>, Box<dyn Error>> {
-			// We are not giving a native call and thus we are sure that the result can never be a
-			// native value.
-			self.execute_using_consensus_failure_handler(strategy.get_manager())
-		}
-
-		fn execute_aux(&mut self, use_native: bool) -> (CallResult<Exec::Error>, bool) {
+		pub fn execute(&mut self) -> Result<Vec<u8>, Box<dyn Error>> {
 			let mut cache = StorageTransactionCache::default();
 
 			let cache = match self.storage_transaction_cache.as_mut() {
@@ -390,7 +295,7 @@ mod execution {
 				.enter_runtime()
 				.expect("StateMachine is never called from the runtime; qed");
 
-			let mut ext = Ext::new(self.overlay, cache, self.backend, Some(&mut self.extensions));
+			let mut ext = Ext::new(self.overlay, cache, self.backend, Some(self.extensions));
 
 			let ext_id = ext.id;
 
@@ -403,14 +308,10 @@ mod execution {
 				"Call",
 			);
 
-			let (result, was_native) = self.exec.call(
-				&mut ext,
-				self.runtime_code,
-				self.method,
-				self.call_data,
-				use_native,
-				self.context,
-			);
+			let result = self
+				.exec
+				.call(&mut ext, self.runtime_code, self.method, self.call_data, false, self.context)
+				.0;
 
 			self.overlay
 				.exit_runtime()
@@ -419,92 +320,11 @@ mod execution {
 			trace!(
 				target: "state",
 				ext_id = %HexDisplay::from(&ext_id.to_le_bytes()),
-				?was_native,
 				?result,
 				"Return",
 			);
 
-			(result, was_native)
-		}
-
-		fn execute_call_with_both_strategy<Handler>(
-			&mut self,
-			on_consensus_failure: Handler,
-		) -> CallResult<Exec::Error>
-		where
-			Handler:
-				FnOnce(CallResult<Exec::Error>, CallResult<Exec::Error>) -> CallResult<Exec::Error>,
-		{
-			self.overlay.start_transaction();
-			let (result, was_native) = self.execute_aux(true);
-
-			if was_native {
-				self.overlay.rollback_transaction().expect(PROOF_CLOSE_TRANSACTION);
-				let (wasm_result, _) = self.execute_aux(false);
-
-				if (result.is_ok() &&
-					wasm_result.is_ok() && result.as_ref().ok() == wasm_result.as_ref().ok()) ||
-					result.is_err() && wasm_result.is_err()
-				{
-					result
-				} else {
-					on_consensus_failure(wasm_result, result)
-				}
-			} else {
-				self.overlay.commit_transaction().expect(PROOF_CLOSE_TRANSACTION);
-				result
-			}
-		}
-
-		fn execute_call_with_native_else_wasm_strategy(&mut self) -> CallResult<Exec::Error> {
-			self.overlay.start_transaction();
-			let (result, was_native) = self.execute_aux(true);
-
-			if !was_native || result.is_ok() {
-				self.overlay.commit_transaction().expect(PROOF_CLOSE_TRANSACTION);
-				result
-			} else {
-				self.overlay.rollback_transaction().expect(PROOF_CLOSE_TRANSACTION);
-				self.execute_aux(false).0
-			}
-		}
-
-		/// Execute a call using the given state backend, overlayed changes, and call executor.
-		///
-		/// On an error, no prospective changes are written to the overlay.
-		///
-		/// Note: changes to code will be in place if this call is made again. For running partial
-		/// blocks (e.g. a transaction at a time), ensure a different method is used.
-		///
-		/// Returns the result of the executed function either in native representation `R` or
-		/// in SCALE encoded representation.
-		pub fn execute_using_consensus_failure_handler<Handler>(
-			&mut self,
-			manager: ExecutionManager<Handler>,
-		) -> Result<Vec<u8>, Box<dyn Error>>
-		where
-			Handler:
-				FnOnce(CallResult<Exec::Error>, CallResult<Exec::Error>) -> CallResult<Exec::Error>,
-		{
-			let result = {
-				match manager {
-					ExecutionManager::Both(on_consensus_failure) =>
-						self.execute_call_with_both_strategy(on_consensus_failure),
-					ExecutionManager::NativeElseWasm =>
-						self.execute_call_with_native_else_wasm_strategy(),
-					ExecutionManager::AlwaysWasm(trust_level) => {
-						let _abort_guard = match trust_level {
-							BackendTrustLevel::Trusted => None,
-							BackendTrustLevel::Untrusted =>
-								Some(sp_panic_handler::AbortGuard::never_abort()),
-						};
-						self.execute_aux(false).0
-					},
-					ExecutionManager::NativeWhenPossible => self.execute_aux(true).0,
-				}
-			};
-
-			result.map_err(|e| Box::new(e) as _)
+			result.map_err(|e| Box::new(e) as Box<_>)
 		}
 	}
 
@@ -531,7 +351,7 @@ mod execution {
 			method,
 			call_data,
 			runtime_code,
-			Default::default(),
+			&mut Default::default(),
 		)
 	}
 
@@ -551,7 +371,7 @@ mod execution {
 		method: &str,
 		call_data: &[u8],
 		runtime_code: &RuntimeCode,
-		extensions: Extensions,
+		extensions: &mut Extensions,
 	) -> Result<(Vec<u8>, StorageProof), Box<dyn Error>>
 	where
 		S: trie_backend_essence::TrieBackendStorage<H>,
@@ -572,7 +392,7 @@ mod execution {
 			runtime_code,
 			CallContext::Offchain,
 		)
-		.execute_using_consensus_failure_handler::<_>(always_wasm())?;
+		.execute()?;
 
 		let proof = proving_backend
 			.extract_proof()
@@ -627,11 +447,11 @@ mod execution {
 			exec,
 			method,
 			call_data,
-			Extensions::default(),
+			&mut Extensions::default(),
 			runtime_code,
 			CallContext::Offchain,
 		)
-		.execute_using_consensus_failure_handler(always_untrusted_wasm())
+		.execute()
 	}
 
 	/// Generate storage read proof.
@@ -1356,6 +1176,7 @@ mod tests {
 		let backend = trie_backend::tests::test_trie(state_version, None, None);
 		let mut overlayed_changes = Default::default();
 		let wasm_code = RuntimeCode::empty();
+		let mut execution_extensions = &mut Default::default();
 
 		let mut state_machine = StateMachine::new(
 			&backend,
@@ -1367,12 +1188,12 @@ mod tests {
 			},
 			"test",
 			&[],
-			Default::default(),
+			&mut execution_extensions,
 			&wasm_code,
 			CallContext::Offchain,
 		);
 
-		assert_eq!(state_machine.execute(ExecutionStrategy::NativeWhenPossible).unwrap(), vec![66]);
+		assert_eq!(state_machine.execute().unwrap(), vec![66]);
 	}
 
 	#[test]
@@ -1384,6 +1205,7 @@ mod tests {
 		let backend = trie_backend::tests::test_trie(state_version, None, None);
 		let mut overlayed_changes = Default::default();
 		let wasm_code = RuntimeCode::empty();
+		let mut execution_extensions = &mut Default::default();
 
 		let mut state_machine = StateMachine::new(
 			&backend,
@@ -1395,47 +1217,12 @@ mod tests {
 			},
 			"test",
 			&[],
-			Default::default(),
+			&mut execution_extensions,
 			&wasm_code,
 			CallContext::Offchain,
 		);
 
-		assert_eq!(state_machine.execute(ExecutionStrategy::NativeElseWasm).unwrap(), vec![66]);
-	}
-
-	#[test]
-	fn dual_execution_strategy_detects_consensus_failure() {
-		dual_execution_strategy_detects_consensus_failure_inner(StateVersion::V0);
-		dual_execution_strategy_detects_consensus_failure_inner(StateVersion::V1);
-	}
-	fn dual_execution_strategy_detects_consensus_failure_inner(state_version: StateVersion) {
-		let mut consensus_failed = false;
-		let backend = trie_backend::tests::test_trie(state_version, None, None);
-		let mut overlayed_changes = Default::default();
-		let wasm_code = RuntimeCode::empty();
-
-		let mut state_machine = StateMachine::new(
-			&backend,
-			&mut overlayed_changes,
-			&DummyCodeExecutor {
-				native_available: true,
-				native_succeeds: true,
-				fallback_succeeds: false,
-			},
-			"test",
-			&[],
-			Default::default(),
-			&wasm_code,
-			CallContext::Offchain,
-		);
-
-		assert!(state_machine
-			.execute_using_consensus_failure_handler(ExecutionManager::Both(|we, _ne| {
-				consensus_failed = true;
-				we
-			}),)
-			.is_err());
-		assert!(consensus_failed);
+		assert_eq!(state_machine.execute().unwrap(), vec![66]);
 	}
 
 	#[test]