Don't include :code by default in storage proofs (#5179)

* Don't include `:code` by default in storage proofs (#5060)

* Adds test to verify that the runtime currently is always contained in
the proof

* Start passing the runtime wasm code from the outside

* Fix compilation

* More build fixes

* Make the test work as expected now :)

* Last fixes

* Fixes benchmarks

* Review feedback

* Apply suggestions from code review

Co-Authored-By: Sergei Pepyakin <sergei@parity.io>

* Review feedback

* Fix compilation

Co-authored-by: Sergei Pepyakin <s.pepyakin@gmail.com>

* Fix compilation and change the way `RuntimeCode` works

* Fix tests

* Switch to `Cow`

Co-authored-by: Benjamin Kampmann <ben@gnunicorn.org>
Co-authored-by: Sergei Pepyakin <s.pepyakin@gmail.com>

substrate/client/src/call_executor.rs

@@ -81,6 +81,7 @@ where
 			id, self.backend.changes_trie_storage()
 		)?;
 		let state = self.backend.state_at(*id)?;
+		let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
 		let return_data = StateMachine::new(
 			&state,
 			changes_trie,
@@ -89,6 +90,7 @@ where
 			method,
 			call_data,
 			extensions.unwrap_or_default(),
+			&state_runtime_code.runtime_code()?,
 		).execute_using_consensus_failure_handler::<_, NeverNativeValue, fn() -> _>(
 			strategy.get_manager(),
 			None,
@@ -135,42 +137,53 @@ where
 		let mut storage_transaction_cache = storage_transaction_cache.map(|c| c.borrow_mut());
 
 		let mut state = self.backend.state_at(*at)?;
-		match recorder {
-			Some(recorder) => state.as_trie_backend()
-				.ok_or_else(||
-					Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof)
-						as Box<dyn sp_state_machine::Error>
-				)
-				.and_then(|trie_state| {
-					let backend = sp_state_machine::ProvingBackend::new_with_recorder(
-						trie_state,
-						recorder.clone(),
-					);
 
-					StateMachine::new(
-						&backend,
-						changes_trie_state,
-						&mut *changes.borrow_mut(),
-						&self.executor,
-						method,
-						call_data,
-						extensions.unwrap_or_default(),
-					)
-					// TODO: https://github.com/paritytech/substrate/issues/4455
-					// .with_storage_transaction_cache(storage_transaction_cache.as_mut().map(|c| &mut **c))
-					.execute_using_consensus_failure_handler(execution_manager, native_call)
-				}),
-			None => StateMachine::new(
-				&state,
-				changes_trie_state,
-				&mut *changes.borrow_mut(),
-				&self.executor,
-				method,
-				call_data,
-				extensions.unwrap_or_default(),
-			)
-			.with_storage_transaction_cache(storage_transaction_cache.as_mut().map(|c| &mut **c))
-			.execute_using_consensus_failure_handler(execution_manager, native_call)
+		match recorder {
+			Some(recorder) => {
+				let trie_state = state.as_trie_backend()
+					.ok_or_else(||
+						Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof) as Box<dyn sp_state_machine::Error>
+					)?;
+
+				let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&trie_state);
+				// It is important to extract the runtime code here before we create the proof
+				// recorder.
+				let runtime_code = state_runtime_code.runtime_code()?;
+
+				let backend = sp_state_machine::ProvingBackend::new_with_recorder(
+					trie_state,
+					recorder.clone(),
+				);
+
+				StateMachine::new(
+					&backend,
+					changes_trie_state,
+					&mut *changes.borrow_mut(),
+					&self.executor,
+					method,
+					call_data,
+					extensions.unwrap_or_default(),
+					&runtime_code,
+				)
+				// TODO: https://github.com/paritytech/substrate/issues/4455
+				// .with_storage_transaction_cache(storage_transaction_cache.as_mut().map(|c| &mut **c))
+				.execute_using_consensus_failure_handler(execution_manager, native_call)
+			},
+			None => {
+				let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
+				StateMachine::new(
+					&state,
+					changes_trie_state,
+					&mut *changes.borrow_mut(),
+					&self.executor,
+					method,
+					call_data,
+					extensions.unwrap_or_default(),
+					&state_runtime_code.runtime_code()?,
+				)
+				.with_storage_transaction_cache(storage_transaction_cache.as_mut().map(|c| &mut **c))
+				.execute_using_consensus_failure_handler(execution_manager, native_call)
+			}
 		}.map_err(Into::into)
 	}
 
@@ -189,7 +202,8 @@ where
 			changes_trie_state,
 			None,
 		);
-		self.executor.runtime_version(&mut ext)
+		let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
+		self.executor.runtime_version(&mut ext, &state_runtime_code.runtime_code()?)
 			.map_err(|e| sp_blockchain::Error::VersionInvalid(format!("{:?}", e)).into())
 	}
 
@@ -206,6 +220,7 @@ where
 			&self.executor,
 			method,
 			call_data,
+			&sp_state_machine::backend::BackendRuntimeCode::new(trie_state).runtime_code()?,
 		)
 		.map_err(Into::into)
 	}

substrate/client/src/client.rs

@@ -41,8 +41,7 @@ use sp_runtime::{
 use sp_state_machine::{
 	DBValue, Backend as StateBackend, ChangesTrieAnchorBlockId,
 	prove_read, prove_child_read, ChangesTrieRootsStorage, ChangesTrieStorage,
-	ChangesTrieConfigurationRange, key_changes, key_changes_proof, StorageProof,
-	merge_storage_proofs,
+	ChangesTrieConfigurationRange, key_changes, key_changes_proof,
 };
 use sc_executor::{RuntimeVersion, RuntimeInfo};
 use sp_consensus::{
@@ -55,6 +54,7 @@ use sp_blockchain::{self as blockchain,
 	well_known_cache_keys::Id as CacheKeyId,
 	HeaderMetadata, CachedHeaderMetadata,
 };
+use sp_trie::StorageProof;
 
 use sp_api::{
 	CallApiAt, ConstructRuntimeApi, Core as CoreApi, ApiExt, ApiRef, ProvideRuntimeApi,
@@ -472,7 +472,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 			Ok(())
 		}, ())?;
 
-		Ok(merge_storage_proofs(proofs))
+		Ok(StorageProof::merge(proofs))
 	}
 
 	/// Generates CHT-based proof for roots of changes tries at given blocks (that are part of single CHT).
@@ -1140,9 +1140,20 @@ impl<B, E, Block, RA> ProofProvider<Block> for Client<B, E, Block, RA> where
 		method: &str,
 		call_data: &[u8]
 	) -> sp_blockchain::Result<(Vec<u8>, StorageProof)> {
+		// Make sure we include the `:code` and `:heap_pages` in the execution proof to be
+		// backwards compatible.
+		//
+		// TODO: Remove when solved: https://github.com/paritytech/substrate/issues/5047
+		let code_proof = self.read_proof(
+			id,
+			&mut [well_known_keys::CODE, well_known_keys::HEAP_PAGES].iter().map(|v| *v),
+		)?;
+
 		let state = self.state_at(id)?;
 		let header = self.prepare_environment_block(id)?;
-		prove_execution(state, header, &self.executor, method, call_data)
+		prove_execution(state, header, &self.executor, method, call_data).map(|(r, p)| {
+			(r, StorageProof::merge(vec![p, code_proof]))
+		})
 	}
 
 	fn header_proof(&self, id: &BlockId<Block>) -> sp_blockchain::Result<(Block::Header, StorageProof)> {

substrate/client/src/genesis.rs

@@ -89,6 +89,8 @@ mod tests {
 		};
 		let hash = header.hash();
 		let mut overlay = OverlayedChanges::default();
+		let backend_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&backend);
+		let runtime_code = backend_runtime_code.runtime_code().expect("Code is part of the backend");
 
 		StateMachine::new(
 			backend,
@@ -98,6 +100,7 @@ mod tests {
 			"Core_initialize_block",
 			&header.encode(),
 			Default::default(),
+			&runtime_code,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		).unwrap();
@@ -111,6 +114,7 @@ mod tests {
 				"BlockBuilder_apply_extrinsic",
 				&tx.encode(),
 				Default::default(),
+				&runtime_code,
 			).execute(
 				ExecutionStrategy::NativeElseWasm,
 			).unwrap();
@@ -124,6 +128,7 @@ mod tests {
 			"BlockBuilder_finalize_block",
 			&[],
 			Default::default(),
+			&runtime_code,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		).unwrap();
@@ -161,6 +166,8 @@ mod tests {
 
 		let backend = InMemoryBackend::from(storage);
 		let (b1data, _b1hash) = block1(genesis_hash, &backend);
+		let backend_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&backend);
+		let runtime_code = backend_runtime_code.runtime_code().expect("Code is part of the backend");
 
 		let mut overlay = OverlayedChanges::default();
 		let _ = StateMachine::new(
@@ -171,6 +178,7 @@ mod tests {
 			"Core_execute_block",
 			&b1data,
 			Default::default(),
+			&runtime_code,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		).unwrap();
@@ -189,6 +197,8 @@ mod tests {
 
 		let backend = InMemoryBackend::from(storage);
 		let (b1data, _b1hash) = block1(genesis_hash, &backend);
+		let backend_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&backend);
+		let runtime_code = backend_runtime_code.runtime_code().expect("Code is part of the backend");
 
 		let mut overlay = OverlayedChanges::default();
 		let _ = StateMachine::new(
@@ -199,6 +209,7 @@ mod tests {
 			"Core_execute_block",
 			&b1data,
 			Default::default(),
+			&runtime_code,
 		).execute(
 			ExecutionStrategy::AlwaysWasm,
 		).unwrap();
@@ -217,6 +228,8 @@ mod tests {
 
 		let backend = InMemoryBackend::from(storage);
 		let (b1data, _b1hash) = block1(genesis_hash, &backend);
+		let backend_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&backend);
+		let runtime_code = backend_runtime_code.runtime_code().expect("Code is part of the backend");
 
 		let mut overlay = OverlayedChanges::default();
 		let r = StateMachine::new(
@@ -227,6 +240,7 @@ mod tests {
 			"Core_execute_block",
 			&b1data,
 			Default::default(),
+			&runtime_code,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		);

substrate/client/src/light/backend.rs

@@ -172,9 +172,9 @@ impl<S, Block> ClientBackend<Block> for Backend<S, HashFor<Block>>
 				match maybe_val {
 					Some(val) => self.blockchain.storage().insert_aux(
 						&[(&key[..], &val[..])],
-						::std::iter::empty(),
+						std::iter::empty(),
 					)?,
-					None => self.blockchain.storage().insert_aux(::std::iter::empty(), &[&key[..]])?,
+					None => self.blockchain.storage().insert_aux(std::iter::empty(), &[&key[..]])?,
 				}
 			}
 		}

substrate/client/src/light/call_executor.rs

@@ -29,7 +29,6 @@ use sp_externalities::Extensions;
 use sp_state_machine::{
 	self, Backend as StateBackend, OverlayedChanges, ExecutionStrategy, create_proof_check_backend,
 	execution_proof_check_on_trie_backend, ExecutionManager, StorageProof,
-	merge_storage_proofs,
 };
 use hash_db::Hasher;
 
@@ -206,7 +205,7 @@ pub fn prove_execution<Block, S, E>(
 		method,
 		call_data,
 	)?;
-	let total_proof = merge_storage_proofs(vec![init_proof, exec_proof]);
+	let total_proof = StorageProof::merge(vec![init_proof, exec_proof]);
 
 	Ok((result, total_proof))
 }
@@ -259,12 +258,18 @@ fn check_execution_proof_with_make_header<Header, E, H, MakeNextHeader: Fn(&Head
 	let mut changes = OverlayedChanges::default();
 	let trie_backend = create_proof_check_backend(root, remote_proof)?;
 	let next_header = make_next_header(&request.header);
+
+	// TODO: Remove when solved: https://github.com/paritytech/substrate/issues/5047
+	let backend_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&trie_backend);
+	let runtime_code = backend_runtime_code.runtime_code()?;
+
 	execution_proof_check_on_trie_backend::<H, Header::Number, _>(
 		&trie_backend,
 		&mut changes,
 		executor,
 		"Core_initialize_block",
 		&next_header.encode(),
+		&runtime_code,
 	)?;
 
 	// execute method
@@ -274,7 +279,9 @@ fn check_execution_proof_with_make_header<Header, E, H, MakeNextHeader: Fn(&Head
 		executor,
 		&request.method,
 		&request.call_data,
-	).map_err(Into::into)
+		&runtime_code,
+	)
+	.map_err(Into::into)
 }
 
 #[cfg(test)]

substrate/client/src/light/fetcher.rs

@@ -30,7 +30,7 @@ use sp_runtime::traits::{
 use sp_state_machine::{
 	ChangesTrieRootsStorage, ChangesTrieAnchorBlockId, ChangesTrieConfigurationRange,
 	InMemoryChangesTrieStorage, TrieBackend, read_proof_check, key_changes_proof_check_with_db,
-	create_proof_check_backend_storage, read_child_proof_check,
+	read_child_proof_check,
 };
 pub use sp_state_machine::StorageProof;
 use sp_blockchain::{Error as ClientError, Result as ClientResult};
@@ -155,7 +155,7 @@ impl<E, H, B: BlockT, S: BlockchainStorage<B>> LightDataChecker<E, H, B, S> {
 			H::Out: Ord + codec::Codec,
 	{
 		// all the checks are sharing the same storage
-		let storage = create_proof_check_backend_storage(remote_roots_proof);
+		let storage = remote_roots_proof.into_memory_db();
 
 		// remote_roots.keys() are sorted => we can use this to group changes tries roots
 		// that are belongs to the same CHT
@@ -187,7 +187,8 @@ impl<E, H, B: BlockT, S: BlockchainStorage<B>> LightDataChecker<E, H, B, S> {
 					local_cht_root,
 					block,
 					remote_changes_trie_root,
-					&proving_backend)?;
+					&proving_backend,
+				)?;
 
 				// and return the storage to use in following checks
 				storage = proving_backend.into_storage();
@@ -270,7 +271,7 @@ impl<E, Block, H, S> FetchChecker<Block> for LightDataChecker<E, H, Block, S>
 		body: Vec<Block::Extrinsic>
 	) -> ClientResult<Vec<Block::Extrinsic>> {
 		// TODO: #2621
-		let	extrinsics_root = HashFor::<Block>::ordered_trie_root(
+		let extrinsics_root = HashFor::<Block>::ordered_trie_root(
 			body.iter().map(Encode::encode).collect(),
 		);
 		if *request.header.extrinsics_root() == extrinsics_root {
@@ -294,7 +295,7 @@ struct RootsStorage<'a, Number: AtLeast32Bit, Hash: 'a> {
 impl<'a, H, Number, Hash> ChangesTrieRootsStorage<H, Number> for RootsStorage<'a, Number, Hash>
 	where
 		H: Hasher,
-		Number: ::std::fmt::Display + ::std::hash::Hash + Clone + AtLeast32Bit + Encode + Decode + Send + Sync + 'static,
+		Number: std::fmt::Display + std::hash::Hash + Clone + AtLeast32Bit + Encode + Decode + Send + Sync + 'static,
 		Hash: 'a + Send + Sync + Clone + AsRef<[u8]>,
 {
 	fn build_anchor(