Benchmarks sanity checks (#6119)

* add read-only externalities

* sanity checks

* cleanup

* Update primitives/state-machine/src/read_only.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* fix typo

* add error exit code if nothing was run

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

bin/node/bench/Cargo.toml

@@ -12,6 +12,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 log = "0.4.8"
 node-primitives = { version = "2.0.0-dev", path = "../primitives" }
 node-testing = { version = "2.0.0-dev", path = "../testing" }
+node-runtime = { version = "2.0.0-dev", path = "../runtime" }
 sc-cli = { version = "0.8.0-dev", path = "../../../client/cli" }
 sc-client-api = { version = "2.0.0-dev", path = "../../../client/api/" }
 sp-runtime = { version = "2.0.0-dev", path = "../../../primitives/runtime" }

bin/node/bench/src/import.rs

@@ -36,6 +36,7 @@ use node_testing::bench::{BenchDb, Profile, BlockType, KeyTypes, DatabaseType};
 use node_primitives::Block;
 use sc_client_api::backend::Backend;
 use sp_runtime::generic::BlockId;
+use sp_state_machine::InspectState;
 
 use crate::core::{self, Path, Mode};
 
@@ -81,6 +82,7 @@ pub struct ImportBenchmark {
 	profile: Profile,
 	database: BenchDb,
 	block: Block,
+	block_type: BlockType,
 }
 
 impl core::BenchmarkDescription for ImportBenchmarkDescription {
@@ -124,6 +126,7 @@ impl core::BenchmarkDescription for ImportBenchmarkDescription {
 		let block = bench_db.generate_block(self.block_type.to_content(self.size.transactions()));
 		Box::new(ImportBenchmark {
 			database: bench_db,
+			block_type: self.block_type,
 			block,
 			profile,
 		})
@@ -155,6 +158,41 @@ impl core::Benchmark for ImportBenchmark {
 		context.import_block(self.block.clone());
 		let elapsed = start.elapsed();
 
+		// Sanity checks.
+		context.client.state_at(&BlockId::number(1)).expect("state_at failed for block#1")
+			.inspect_with(|| {
+				match self.block_type {
+					BlockType::RandomTransfersKeepAlive => {
+						// should be 5 per signed extrinsic + 1 per unsigned
+						// we have 1 unsigned and the rest are signed in the block
+						// those 5 events per signed are:
+						//    - new account (RawEvent::NewAccount) as we always transfer fund to non-existant account
+						//    - endowed (RawEvent::Endowed) for this new account
+						//    - successful transfer (RawEvent::Transfer) for this transfer operation
+						//    - deposit event for charging transaction fee
+						//    - extrinsic success
+						assert_eq!(
+							node_runtime::System::events().len(),
+							(self.block.extrinsics.len() - 1) * 5 + 1,
+						);
+					},
+					BlockType::Noop => {
+						assert_eq!(
+							node_runtime::System::events().len(),
+
+							// should be 2 per signed extrinsic + 1 per unsigned
+							// we have 1 unsigned and the rest are signed in the block
+							// those 2 events per signed are:
+							//    - deposit event for charging transaction fee
+							//    - extrinsic success
+							(self.block.extrinsics.len() - 1) *  2 + 1,
+						);
+					},
+					_ => {},
+				}
+			}
+		);
+
 		if mode == Mode::Profile {
 			std::thread::park_timeout(std::time::Duration::from_secs(1));
 		}

bin/node/bench/src/main.rs

@@ -152,6 +152,11 @@ fn main() {
 		}
 	}
 
+	if results.is_empty() {
+		eprintln!("No benchmark was found for query");
+		std::process::exit(1);
+	}
+
 	if opt.json {
 		let json_result: String = serde_json::to_string(&results).expect("Failed to construct json");
 		println!("{}", json_result);

primitives/state-machine/src/lib.rs

@@ -42,10 +42,12 @@ mod proving_backend;
 mod trie_backend;
 mod trie_backend_essence;
 mod stats;
+mod read_only;
 
 pub use sp_trie::{trie_types::{Layout, TrieDBMut}, StorageProof, TrieMut, DBValue, MemoryDB};
 pub use testing::TestExternalities;
 pub use basic::BasicExternalities;
+pub use read_only::{ReadOnlyExternalities, InspectState};
 pub use ext::Ext;
 pub use backend::Backend;
 pub use changes_trie::{

primitives/state-machine/src/read_only.rs

@@ -0,0 +1,194 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2020 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Read-only version of Externalities.
+
+use std::{
+	any::{TypeId, Any},
+	marker::PhantomData,
+};
+use crate::{Backend, StorageKey, StorageValue};
+use hash_db::Hasher;
+use sp_core::{
+	storage::ChildInfo,
+	traits::Externalities, Blake2Hasher,
+};
+use codec::Encode;
+
+/// Trait for inspecting state in any backend.
+///
+/// Implemented for any backend.
+pub trait InspectState<H: Hasher, B: Backend<H>> {
+	/// Inspect state with a closure.
+	///
+	/// Self will be set as read-only externalities and inspection
+	/// closure will be run against it.
+	fn inspect_with<F: FnOnce()>(&self, f: F);
+}
+
+impl<H: Hasher, B: Backend<H>> InspectState<H, B> for B {
+	fn inspect_with<F: FnOnce()>(&self, f: F) {
+		ReadOnlyExternalities::from(self).execute_with(f)
+	}
+}
+
+/// Simple read-only externalities for any backend.
+///
+/// To be used in test for state inspection. Will panic if something writes
+/// to the storage.
+#[derive(Debug)]
+pub struct ReadOnlyExternalities<'a, H: Hasher, B: 'a + Backend<H>> {
+	backend: &'a B,
+	_phantom: PhantomData<H>,
+}
+
+impl<'a, H: Hasher, B: 'a + Backend<H>> From<&'a B> for ReadOnlyExternalities<'a, H, B> {
+	fn from(backend: &'a B) -> Self {
+		ReadOnlyExternalities { backend, _phantom: PhantomData }
+	}
+}
+
+impl<'a, H: Hasher, B: 'a + Backend<H>> ReadOnlyExternalities<'a, H, B> {
+	/// Execute the given closure while `self` is set as externalities.
+	///
+	/// Returns the result of the given closure.
+	pub fn execute_with<R>(&mut self, f: impl FnOnce() -> R) -> R {
+		sp_externalities::set_and_run_with_externalities(self, f)
+	}
+}
+
+impl<'a, H: Hasher, B: 'a + Backend<H>> Externalities for ReadOnlyExternalities<'a, H, B> {
+	fn set_offchain_storage(&mut self, _key: &[u8], _value: Option<&[u8]>) {
+		panic!("Should not be used in read-only externalities!")
+	}
+
+	fn storage(&self, key: &[u8]) -> Option<StorageValue> {
+		self.backend.storage(key).expect("Backed failed for storage in ReadOnlyExternalities")
+	}
+
+	fn storage_hash(&self, key: &[u8]) -> Option<Vec<u8>> {
+		self.storage(key).map(|v| Blake2Hasher::hash(&v).encode())
+	}
+
+	fn child_storage(
+		&self,
+		child_info: &ChildInfo,
+		key: &[u8],
+	) -> Option<StorageValue> {
+		self.backend.child_storage(child_info, key).expect("Backed failed for child_storage in ReadOnlyExternalities")
+	}
+
+	fn child_storage_hash(
+		&self,
+		child_info: &ChildInfo,
+		key: &[u8],
+	) -> Option<Vec<u8>> {
+		self.child_storage(child_info, key).map(|v| Blake2Hasher::hash(&v).encode())
+	}
+
+	fn next_storage_key(&self, key: &[u8]) -> Option<StorageKey> {
+		self.backend.next_storage_key(key).expect("Backed failed for next_storage_key in ReadOnlyExternalities")
+	}
+
+	fn next_child_storage_key(
+		&self,
+		child_info: &ChildInfo,
+		key: &[u8],
+	) -> Option<StorageKey> {
+		self.backend.next_child_storage_key(child_info, key)
+			.expect("Backed failed for next_child_storage_key in ReadOnlyExternalities")
+	}
+
+	fn place_storage(&mut self, _key: StorageKey, _maybe_value: Option<StorageValue>) {
+		unimplemented!("place_storage not supported in ReadOnlyExternalities")
+	}
+
+	fn place_child_storage(
+		&mut self,
+		_child_info: &ChildInfo,
+		_key: StorageKey,
+		_value: Option<StorageValue>,
+	) {
+		unimplemented!("place_child_storage not supported in ReadOnlyExternalities")
+	}
+
+	fn kill_child_storage(
+		&mut self,
+		_child_info: &ChildInfo,
+	) {
+		unimplemented!("kill_child_storage is not supported in ReadOnlyExternalities")
+	}
+
+	fn clear_prefix(&mut self, _prefix: &[u8]) {
+		unimplemented!("clear_prefix is not supported in ReadOnlyExternalities")
+	}
+
+	fn clear_child_prefix(
+		&mut self,
+		_child_info: &ChildInfo,
+		_prefix: &[u8],
+	) {
+		unimplemented!("clear_child_prefix is not supported in ReadOnlyExternalities")
+	}
+
+	fn storage_append(
+		&mut self,
+		_key: Vec<u8>,
+		_value: Vec<u8>,
+	) {
+		unimplemented!("storage_append is not supported in ReadOnlyExternalities")
+	}
+
+	fn chain_id(&self) -> u64 { 42 }
+
+	fn storage_root(&mut self) -> Vec<u8> {
+		unimplemented!("storage_root is not supported in ReadOnlyExternalities")
+	}
+
+	fn child_storage_root(
+		&mut self,
+		_child_info: &ChildInfo,
+	) -> Vec<u8> {
+		unimplemented!("child_storage_root is not supported in ReadOnlyExternalities")
+	}
+
+	fn storage_changes_root(&mut self, _parent: &[u8]) -> Result<Option<Vec<u8>>, ()> {
+		unimplemented!("storage_changes_root is not supported in ReadOnlyExternalities")
+	}
+
+	fn wipe(&mut self) {}
+
+	fn commit(&mut self) {}
+}
+
+impl<'a, H: Hasher, B: 'a + Backend<H>> sp_externalities::ExtensionStore for ReadOnlyExternalities<'a, H, B> {
+	fn extension_by_type_id(&mut self, _type_id: TypeId) -> Option<&mut dyn Any> {
+		unimplemented!("extension_by_type_id is not supported in ReadOnlyExternalities")
+	}
+
+	fn register_extension_with_type_id(
+		&mut self,
+		_type_id: TypeId,
+		_extension: Box<dyn sp_externalities::Extension>,
+	) -> Result<(), sp_externalities::Error> {
+		unimplemented!("register_extension_with_type_id is not supported in ReadOnlyExternalities")
+	}
+
+	fn deregister_extension_by_type_id(&mut self, _type_id: TypeId) -> Result<(), sp_externalities::Error> {
+		unimplemented!("deregister_extension_by_type_id is not supported in ReadOnlyExternalities")
+	}
+}