Batch signature verification (#5023)

* create parallel tasks extension * make type system happy * basic externalities * test for dynamic extensions * batching test * remove premature verify_batch * shnschnorrkel batch * alter test * shnschnorrkel test * executive batching * some docs * also multi/any signatgures * error propagation * styling * make verification extension optional * experimental ed25519 parallelization * some merge fallout * utilize task executor * merge fallout * utilize task executor more * another merge fallout * feature-gate sp-io * arrange toml * fix no-std * sr25519 batching and refactoring * add docs * fix name * add newline * fix block import test * long sr25519 test * blocking instead of parking * move everything in crypto * return batch_verify to check :) * use condvars * use multi-threaded executor for benches * don't call via host interface * try no spawning * add true * cleanup * straighten batching * remove signature check from this test (?) * remove now pointless test * remove another now useless test * fix warnings * Revert "remove another now useless test" This reverts commit bbdec24. * rethink the sp-io-part * Revert "remove now pointless test" This reverts commit 4d55306. * fix wording * add wording * add todo and fix * return check and fix * add logging in sp-io * Update primitives/io/src/batch_verifier.rs Co-Authored-By: cheme <[email protected]> * address review and use std condvar * account for early exit * address reivew * address review * more suggestions * add docs for batch verification * remove unused * more review suggestions * move to sp-runtime * add expects * remove blocks * use entry * Update primitives/io/src/batch_verifier.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update primitives/externalities/src/extensions.rs Co-Authored-By: Bastian Köcher <[email protected]> * update overlooked note * remove stupid return * Update primitives/io/src/lib.rs Co-Authored-By: Bastian Köcher <[email protected]> * Update primitives/io/src/lib.rs Co-Authored-By: Bastian Köcher <[email protected]> * fix wording * bump spec_version Co-authored-by: cheme <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> # Conflicts: # bin/node/runtime/src/lib.rs # frame/executive/Cargo.toml
paritytech · Apr 17, 2020 · 3940128 · 3940128
1 parent 0132d52
commit 3940128
Show file tree

Hide file tree

Showing 21 changed files with 712 additions and 48 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
@@ -127,8 +127,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 240,
-	impl_version: 1,
+	spec_version: 242,
+	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 };
 

diff --git a/bin/node/testing/Cargo.toml b/bin/node/testing/Cargo.toml
@@ -47,6 +47,7 @@ sp-blockchain = { version = "2.0.0-alpha.5", path = "../../../primitives/blockch
 log = "0.4.8"
 tempfile = "3.1.0"
 fs_extra = "1"
+futures = "0.3.1"
 
 [dev-dependencies]
 criterion = "0.3.0"

diff --git a/bin/node/testing/src/bench.rs b/bin/node/testing/src/bench.rs
@@ -47,7 +47,7 @@ use node_runtime::{
 	AccountId,
 	Signature,
 };
-use sp_core::{ExecutionContext, blake2_256};
+use sp_core::{ExecutionContext, blake2_256, traits::CloneableSpawn};
 use sp_api::ProvideRuntimeApi;
 use sp_block_builder::BlockBuilder;
 use sp_inherents::InherentData;
@@ -57,6 +57,7 @@ use sc_client_api::{
 };
 use sp_core::{Pair, Public, sr25519, ed25519};
 use sc_block_builder::BlockBuilderProvider;
+use futures::{executor, task};
 
 /// Keyring full of accounts for benching.
 ///
@@ -142,6 +143,36 @@ impl BlockType {
 	}
 }
 
+/// Benchmarking task executor.
+///
+/// Uses multiple threads as the regular executable.
+#[derive(Debug, Clone)]
+pub struct TaskExecutor {
+	pool: executor::ThreadPool,
+}
+
+impl TaskExecutor {
+	fn new() -> Self {
+		Self {
+			pool: executor::ThreadPool::new()
+				.expect("Failed to create task executor")
+		}
+	}
+}
+
+impl task::Spawn for TaskExecutor {
+	fn spawn_obj(&self, future: task::FutureObj<'static, ()>)
+	-> Result<(), task::SpawnError> {
+		self.pool.spawn_obj(future)
+	}
+}
+
+impl CloneableSpawn for TaskExecutor {
+	fn clone(&self) -> Box<dyn CloneableSpawn> {
+		Box::new(Clone::clone(self))
+	}
+}
+
 impl BenchDb {
 	/// New immutable benchmarking database.
 	///
@@ -168,8 +199,8 @@ impl BenchDb {
 	/// and keep it there until struct is dropped.
 	///
 	/// You can `clone` this database or you can `create_context` from it
-	/// (which also do `clone`) to run actual operation against new database
-	/// which will be identical to this.
+	/// (which also does `clone`) to run actual operation against new database
+	/// which will be identical to the original.
 	pub fn new(keyring_length: usize) -> Self {
 		Self::with_key_types(keyring_length, KeyTypes::Sr25519)
 	}
@@ -197,7 +228,7 @@ impl BenchDb {
 			None,
 			None,
 			ExecutionExtensions::new(profile.into_execution_strategies(), None),
-			sp_core::tasks::executor(),
+			Box::new(TaskExecutor::new()),
 			None,
 		).expect("Should not fail");
 

diff --git a/client/transaction-pool/src/testing/pool.rs b/client/transaction-pool/src/testing/pool.rs
@@ -20,7 +20,7 @@ use futures::executor::block_on;
 use txpool::{self, Pool};
 use sp_runtime::{
 	generic::BlockId,
-	transaction_validity::{ValidTransaction, InvalidTransaction, TransactionSource},
+	transaction_validity::{ValidTransaction, TransactionSource, InvalidTransaction},
 };
 use substrate_test_runtime_client::{
 	runtime::{Block, Hash, Index, Header, Extrinsic, Transfer},
@@ -263,7 +263,6 @@ fn should_not_retain_invalid_hashes_from_retracted() {
 	let event = block_event_with_retracted(1, vec![retracted_hash]);
 
 	block_on(pool.maintain(event));
-	// maintenance is in background
 	block_on(notifier.next());
 
 	assert_eq!(pool.status().ready, 0);
@@ -701,6 +700,6 @@ fn should_not_accept_old_signatures() {
 		Err(error::Error::Pool(
 			sp_transaction_pool::error::Error::InvalidTransaction(InvalidTransaction::BadProof)
 		)),
-		"Should be invalid transactiono with bad proof",
+		"Should be invalid transaction with bad proof",
 	);
 }
diff --git a/frame/executive/Cargo.toml b/frame/executive/Cargo.toml
@@ -15,6 +15,7 @@ frame-system = { version = "2.0.0-alpha.5", default-features = false, path = "..
 serde = { version = "1.0.101", optional = true }
 sp-runtime = { version = "2.0.0-alpha.5", default-features = false, path = "../../primitives/runtime" }
 sp-std = { version = "2.0.0-alpha.5", default-features = false, path = "../../primitives/std" }
+sp-io = { version = "2.0.0-alpha.5", default-features = false, path = "../../primitives/io" }
 
 [dev-dependencies]
 hex-literal = "0.2.1"

diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs
@@ -237,9 +237,13 @@ where
 		// any initial checks
 		Self::initial_checks(&block);
 
+		let batching_safeguard = sp_runtime::SignatureBatching::start();
 		// execute extrinsics
 		let (header, extrinsics) = block.deconstruct();
 		Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number());
+		if !sp_runtime::SignatureBatching::verify(batching_safeguard) {
+			panic!("Signature verification failed.");
+		}
 
 		// any final checks
 		Self::final_checks(&header);

diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml
@@ -44,6 +44,7 @@ sha2 = { version = "0.8.0", default-features = false, optional = true }
 hex = { version = "0.4", default-features = false, optional = true }
 twox-hash = { version = "1.5.0", default-features = false, optional = true }
 libsecp256k1 = { version = "0.3.2", default-features = false, features = ["hmac"], optional = true }
+merlin = { version = "2.0", default-features = false, optional = true }
 
 sp-runtime-interface = { version = "2.0.0-alpha.5", default-features = false, path = "../runtime-interface" }
 
@@ -94,7 +95,6 @@ std = [
 	"schnorrkel/std",
 	"regex",
 	"num-traits/std",
-	"libsecp256k1/std",
 	"tiny-keccak",
 	"sp-debug-derive/std",
 	"sp-externalities",
@@ -103,6 +103,7 @@ std = [
 	"zeroize/alloc",
 	"futures",
 	"futures/thread-pool",
+	"libsecp256k1/std",
 ]
 
 # This feature enables all crypto primitives for `no_std` builds like microcontrollers
@@ -118,6 +119,7 @@ full_crypto = [
 	"twox-hash",
 	"libsecp256k1",
 	"sp-runtime-interface/disable_target_static_assertions",
+	"merlin",
 ]
 
 [package.metadata.docs.rs]

diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs
@@ -611,6 +611,45 @@ impl CryptoType for Pair {
 	type Pair = Pair;
 }
 
+/// Batch verification.
+///
+/// `messages`, `signatures` and `pub_keys` should all have equal length.
+///
+/// Returns `true` if all signatures are correct, `false` otherwise.
+#[cfg(feature = "std")]
+pub fn verify_batch(
+	messages: Vec<&[u8]>,
+	signatures: Vec<&Signature>,
+	pub_keys: Vec<&Public>,
+) -> bool {
+	let mut sr_pub_keys = Vec::with_capacity(pub_keys.len());
+	for pub_key in pub_keys {
+		match schnorrkel::PublicKey::from_bytes(pub_key.as_ref()) {
+			Ok(pk) => sr_pub_keys.push(pk),
+			Err(_) => return false,
+		};
+	}
+
+	let mut sr_signatures = Vec::with_capacity(signatures.len());
+	for signature in signatures {
+		match schnorrkel::Signature::from_bytes(signature.as_ref()) {
+			Ok(s) => sr_signatures.push(s),
+			Err(_) => return false
+		};
+	}
+
+	let mut messages: Vec<merlin::Transcript> = messages.into_iter().map(
+		|msg| signing_context(SIGNING_CTX).bytes(msg)
+	).collect();
+
+	schnorrkel::verify_batch(
+		&mut messages,
+		&sr_signatures,
+		&sr_pub_keys,
+		true,
+	).is_ok()
+}
+
 #[cfg(test)]
 mod compatibility_test {
 	use super::*;

diff --git a/primitives/externalities/src/extensions.rs b/primitives/externalities/src/extensions.rs
@@ -21,7 +21,8 @@
 //!
 //! It is required that each extension implements the [`Extension`] trait.
 
-use std::{collections::HashMap, any::{Any, TypeId}, ops::DerefMut};
+use std::{collections::HashMap, collections::hash_map::Entry, any::{Any, TypeId}, ops::DerefMut};
+use crate::Error;
 
 /// Marker trait for types that should be registered as [`Externalities`](crate::Externalities) extension.
 ///
@@ -87,6 +88,16 @@ pub trait ExtensionStore {
 	/// It is advised to use [`ExternalitiesExt::extension`](crate::ExternalitiesExt::extension)
 	/// instead of this function to get type system support and automatic type downcasting.
 	fn extension_by_type_id(&mut self, type_id: TypeId) -> Option<&mut dyn Any>;
+
+	/// Register extension `extension` with speciifed `type_id`.
+	///
+	/// It should return error if extension is already registered.
+	fn register_extension_with_type_id(&mut self, type_id: TypeId, extension: Box<dyn Extension>) -> Result<(), Error>;
+
+	/// Deregister extension with speicifed 'type_id' and drop it.
+	///
+	/// It should return error if extension is not registered.
+	fn deregister_extension_by_type_id(&mut self, type_id: TypeId) -> Result<(), Error>;
 }
 
 /// Stores extensions that should be made available through the externalities.
@@ -95,6 +106,12 @@ pub struct Extensions {
 	extensions: HashMap<TypeId, Box<dyn Extension>>,
 }
 
+impl std::fmt::Debug for Extensions {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Extensions: ({})", self.extensions.len())
+    }
+}
+
 impl Extensions {
 	/// Create new instance of `Self`.
 	pub fn new() -> Self {
@@ -106,10 +123,23 @@ impl Extensions {
 		self.extensions.insert(ext.type_id(), Box::new(ext));
 	}
 
+	/// Register extension `ext`.
+	pub fn register_with_type_id(&mut self, type_id: TypeId, extension: Box<dyn Extension>) -> Result<(), Error> {
+		match self.extensions.entry(type_id) {
+			Entry::Vacant(vacant) => { vacant.insert(extension); Ok(()) },
+			Entry::Occupied(_) => Err(Error::ExtensionAlreadyRegistered),
+		}
+	}
+
 	/// Return a mutable reference to the requested extension.
 	pub fn get_mut(&mut self, ext_type_id: TypeId) -> Option<&mut dyn Any> {
 		self.extensions.get_mut(&ext_type_id).map(DerefMut::deref_mut).map(Extension::as_mut_any)
 	}
+
+	/// Deregister extension of type `E`.
+	pub fn deregister(&mut self, type_id: TypeId) -> Option<Box<dyn Extension>> {
+		self.extensions.remove(&type_id)
+	}
 }
 
 #[cfg(test)]

diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs
@@ -32,6 +32,17 @@ pub use extensions::{Extension, Extensions, ExtensionStore};
 mod extensions;
 mod scope_limited;
 
+/// Externalities error.
+#[derive(Debug)]
+pub enum Error {
+	/// Same extension cannot be registered twice.
+	ExtensionAlreadyRegistered,
+	/// Extensions are not supported.
+	ExtensionsAreNotSupported,
+	/// Extension `TypeId` is not registered.
+	ExtensionIsNotRegistered(TypeId),
+}
+
 /// The Substrate externalities.
 ///
 /// Provides access to the storage and to other registered extensions.
@@ -198,10 +209,29 @@ pub trait Externalities: ExtensionStore {
 pub trait ExternalitiesExt {
 	/// Tries to find a registered extension and returns a mutable reference.
 	fn extension<T: Any + Extension>(&mut self) -> Option<&mut T>;
+
+	/// Register extension `ext`.
+	///
+	/// Should return error if extension is already registered or extensions are not supported.
+	fn register_extension<T: Extension>(&mut self, ext: T) -> Result<(), Error>;
+
+	/// Deregister and drop extension of `T` type.
+	///
+	/// Should return error if extension of type `T` is not registered or
+	/// extensions are not supported.
+	fn deregister_extension<T: Extension>(&mut self) -> Result<(), Error>;
 }
 
 impl ExternalitiesExt for &mut dyn Externalities {
 	fn extension<T: Any + Extension>(&mut self) -> Option<&mut T> {
 		self.extension_by_type_id(TypeId::of::<T>()).and_then(Any::downcast_mut)
 	}
+
+	fn register_extension<T: Extension>(&mut self, ext: T) -> Result<(), Error> {
+		self.register_extension_with_type_id(TypeId::of::<T>(), Box::new(ext))
+	}
+
+	fn deregister_extension<T: Extension>(&mut self) -> Result<(), Error> {
+		self.deregister_extension_by_type_id(TypeId::of::<T>())
+	}
 }
diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml
@@ -22,6 +22,8 @@ sp-runtime-interface = { version = "2.0.0-alpha.5", default-features = false, pa
 sp-trie = { version = "2.0.0-alpha.5", optional = true, path = "../../primitives/trie" }
 sp-externalities = { version = "0.8.0-alpha.5", optional = true, path = "../externalities" }
 log = { version = "0.4.8", optional = true }
+futures = { version = "0.3.1", features = ["thread-pool"], optional = true }
+parking_lot = { version = "0.10.0", optional = true }
 
 [features]
 default = ["std"]
@@ -37,6 +39,8 @@ std = [
 	"sp-externalities",
 	"sp-wasm-interface/std",
 	"log",
+	"futures",
+	"parking_lot",
 ]
 
 # These two features are used for `no_std` builds for the environments which already provides