From b680e0dcb2d5f1320e4dc5cd13d328bc96dc11bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 2 Apr 2023 00:03:07 +0200 Subject: [PATCH] Remove deprecated batch verification This removes the deprecated batch verification. This was actually never really activated. Nevertheless, we need to keep the host functions around to support old runtimes which may import these host functions. However, we do not give access to these functions anymore. This means that any new runtime can not call them anymore. The host function implementations we keep will not do batch verification and will instead fall back to the always existing option of directly verifying the passed signature. `finish_batch_verification` will return the combined result of all the batch verify calls. This removes the `TaskExecutorExt` which only existed to support the batch verification. So, any code that used this extension can just remove the registration of them. It also removes `SignatureBatching` that was used by `frame-executive` to control the batch verification. However, there wasn't any `Verify` implementation that called the batch verification functions. --- client/service/src/builder.rs | 1 - client/service/src/client/call_executor.rs | 11 +- client/service/src/client/client.rs | 9 +- client/service/test/src/client/mod.rs | 7 - frame/executive/src/lib.rs | 6 - primitives/api/test/tests/runtime_calls.rs | 1 - primitives/core/src/traits.rs | 12 - primitives/io/src/batch_verifier.rs | 211 ---------------- primitives/io/src/lib.rs | 228 +++++------------- primitives/runtime/src/lib.rs | 66 +---- primitives/state-machine/src/lib.rs | 36 +-- primitives/state-machine/src/testing.rs | 7 +- test-utils/client/src/lib.rs | 1 - .../benchmarking-cli/src/pallet/command.rs | 4 - utils/frame/try-runtime/cli/src/lib.rs | 6 +- 15 files changed, 76 insertions(+), 530 deletions(-) delete mode 100644 primitives/io/src/batch_verifier.rs diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index b04228e6bfc34..5d639431f427b 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -272,7 +272,6 @@ where let executor = crate::client::LocalCallExecutor::new( backend.clone(), executor, - spawn_handle.clone(), config.clone(), execution_extensions, )?; diff --git a/client/service/src/client/call_executor.rs b/client/service/src/client/call_executor.rs index 82d3e36ac80ea..4d019be908b86 100644 --- a/client/service/src/client/call_executor.rs +++ b/client/service/src/client/call_executor.rs @@ -23,7 +23,7 @@ use sc_client_api::{ use sc_executor::{RuntimeVersion, RuntimeVersionOf}; use sp_api::{ProofRecorder, StorageTransactionCache}; use sp_core::{ - traits::{CallContext, CodeExecutor, RuntimeCode, SpawnNamed}, + traits::{CallContext, CodeExecutor, RuntimeCode}, ExecutionContext, }; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; @@ -39,7 +39,6 @@ pub struct LocalCallExecutor { executor: E, wasm_override: Arc>, wasm_substitutes: WasmSubstitutes, - spawn_handle: Box, execution_extensions: Arc>, } @@ -52,7 +51,6 @@ where pub fn new( backend: Arc, executor: E, - spawn_handle: Box, client_config: ClientConfig, execution_extensions: ExecutionExtensions, ) -> sp_blockchain::Result { @@ -72,7 +70,6 @@ where backend, executor, wasm_override: Arc::new(wasm_override), - spawn_handle, wasm_substitutes, execution_extensions: Arc::new(execution_extensions), }) @@ -142,7 +139,6 @@ where backend: self.backend.clone(), executor: self.executor.clone(), wasm_override: self.wasm_override.clone(), - spawn_handle: self.spawn_handle.clone(), wasm_substitutes: self.wasm_substitutes.clone(), execution_extensions: self.execution_extensions.clone(), } @@ -196,7 +192,6 @@ where call_data, extensions, &runtime_code, - self.spawn_handle.clone(), context, ) .set_parent_hash(at_hash); @@ -256,7 +251,6 @@ where call_data, extensions, &runtime_code, - self.spawn_handle.clone(), call_context, ) .with_storage_transaction_cache(storage_transaction_cache.as_deref_mut()) @@ -272,7 +266,6 @@ where call_data, extensions, &runtime_code, - self.spawn_handle.clone(), call_context, ) .with_storage_transaction_cache(storage_transaction_cache.as_deref_mut()) @@ -313,7 +306,6 @@ where trie_backend, &mut Default::default(), &self.executor, - self.spawn_handle.clone(), method, call_data, &runtime_code, @@ -425,7 +417,6 @@ mod tests { backend: backend.clone(), executor: executor.clone(), wasm_override: Arc::new(Some(overrides)), - spawn_handle: Box::new(TaskExecutor::new()), wasm_substitutes: WasmSubstitutes::new( Default::default(), executor.clone(), diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index d5212021f2d79..3adb6d8976969 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -245,13 +245,8 @@ where sc_offchain::OffchainDb::factory_from_backend(&*backend), ); - let call_executor = LocalCallExecutor::new( - backend.clone(), - executor, - spawn_handle.clone(), - config.clone(), - extensions, - )?; + let call_executor = + LocalCallExecutor::new(backend.clone(), executor, config.clone(), extensions)?; Client::new( backend, diff --git a/client/service/test/src/client/mod.rs b/client/service/test/src/client/mod.rs index 96a0a8fe2a023..e6545abf1bc73 100644 --- a/client/service/test/src/client/mod.rs +++ b/client/service/test/src/client/mod.rs @@ -104,7 +104,6 @@ fn construct_block( 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"); - let task_executor = Box::new(TaskExecutor::new()); StateMachine::new( backend, @@ -114,7 +113,6 @@ fn construct_block( &header.encode(), Default::default(), &runtime_code, - task_executor.clone() as Box<_>, CallContext::Onchain, ) .execute(ExecutionStrategy::NativeElseWasm) @@ -129,7 +127,6 @@ fn construct_block( &tx.encode(), Default::default(), &runtime_code, - task_executor.clone() as Box<_>, CallContext::Onchain, ) .execute(ExecutionStrategy::NativeElseWasm) @@ -144,7 +141,6 @@ fn construct_block( &[], Default::default(), &runtime_code, - task_executor.clone() as Box<_>, CallContext::Onchain, ) .execute(ExecutionStrategy::NativeElseWasm) @@ -217,7 +213,6 @@ fn construct_genesis_should_work_with_native() { &b1data, Default::default(), &runtime_code, - TaskExecutor::new(), CallContext::Onchain, ) .execute(ExecutionStrategy::NativeElseWasm) @@ -251,7 +246,6 @@ fn construct_genesis_should_work_with_wasm() { &b1data, Default::default(), &runtime_code, - TaskExecutor::new(), CallContext::Onchain, ) .execute(ExecutionStrategy::AlwaysWasm) @@ -285,7 +279,6 @@ fn construct_genesis_with_bad_transaction_should_panic() { &b1data, Default::default(), &runtime_code, - TaskExecutor::new(), CallContext::Onchain, ) .execute(ExecutionStrategy::NativeElseWasm); diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 5003920eadcb5..6a1041a28e31b 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -480,16 +480,10 @@ where // any initial checks Self::initial_checks(&block); - let signature_batching = sp_runtime::SignatureBatching::start(); - // execute extrinsics let (header, extrinsics) = block.deconstruct(); Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number()); - if !signature_batching.verify() { - panic!("Signature verification failed."); - } - // any final checks Self::final_checks(&header); } diff --git a/primitives/api/test/tests/runtime_calls.rs b/primitives/api/test/tests/runtime_calls.rs index 69f9a88ffadb3..b57793aad1253 100644 --- a/primitives/api/test/tests/runtime_calls.rs +++ b/primitives/api/test/tests/runtime_calls.rs @@ -188,7 +188,6 @@ fn record_proof_works() { &backend, &mut overlay, &executor, - sp_core::testing::TaskExecutor::new(), "Core_execute_block", &block.encode(), &runtime_code, diff --git a/primitives/core/src/traits.rs b/primitives/core/src/traits.rs index 091b1cdb14a34..51327868474a0 100644 --- a/primitives/core/src/traits.rs +++ b/primitives/core/src/traits.rs @@ -169,18 +169,6 @@ impl ReadRuntimeVersionExt { } } -sp_externalities::decl_extension! { - /// Task executor extension. - pub struct TaskExecutorExt(Box); -} - -impl TaskExecutorExt { - /// New instance of task executor extension. - pub fn new(spawn_handle: impl SpawnNamed + Send + 'static) -> Self { - Self(Box::new(spawn_handle)) - } -} - /// Something that can spawn tasks (blocking and non-blocking) with an assigned name /// and optional group. #[dyn_clonable::clonable] diff --git a/primitives/io/src/batch_verifier.rs b/primitives/io/src/batch_verifier.rs deleted file mode 100644 index e6d8c6131f778..0000000000000 --- a/primitives/io/src/batch_verifier.rs +++ /dev/null @@ -1,211 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 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. - -//! Batch/parallel verification. - -use futures::{channel::oneshot, future::FutureExt}; -use sp_core::{crypto::Pair, ecdsa, ed25519, sr25519, traits::SpawnNamed}; -use std::sync::{ - atomic::{AtomicBool, Ordering as AtomicOrdering}, - Arc, -}; - -#[derive(Debug, Clone)] -struct Sr25519BatchItem { - signature: sr25519::Signature, - pub_key: sr25519::Public, - message: Vec, -} - -/// Batch verifier. -/// -/// Used to parallel-verify signatures for runtime host. Provide task executor and -/// just push (`push_ed25519`, `push_sr25519`) as many signature as you need. At the end, -/// call `verify_and_clear to get a result. After that, batch verifier is ready for the -/// next batching job. -pub struct BatchVerifier { - scheduler: Box, - sr25519_items: Vec, - invalid: Arc, - pending_tasks: Vec>, -} - -impl BatchVerifier { - pub fn new(scheduler: Box) -> Self { - BatchVerifier { - scheduler, - sr25519_items: Default::default(), - invalid: Arc::new(false.into()), - pending_tasks: vec![], - } - } - - /// Spawn a verification task. - /// - /// Returns `false` if there was already an invalid verification or if - /// the verification could not be spawned. - fn spawn_verification_task( - &mut self, - f: impl FnOnce() -> bool + Send + 'static, - name: &'static str, - ) -> bool { - // there is already invalid transaction encountered - if self.invalid.load(AtomicOrdering::Relaxed) { - return false - } - - let invalid_clone = self.invalid.clone(); - let (sender, receiver) = oneshot::channel(); - self.pending_tasks.push(receiver); - - self.scheduler.spawn( - name, - None, - async move { - if !f() { - invalid_clone.store(true, AtomicOrdering::Relaxed); - } - if sender.send(()).is_err() { - // sanity - log::warn!("Verification halted while result was pending"); - invalid_clone.store(true, AtomicOrdering::Relaxed); - } - } - .boxed(), - ); - - true - } - - /// Push ed25519 signature to verify. - /// - /// Returns false if some of the pushed signatures before already failed the check - /// (in this case it won't verify anything else) - pub fn push_ed25519( - &mut self, - signature: ed25519::Signature, - pub_key: ed25519::Public, - message: Vec, - ) -> bool { - self.spawn_verification_task( - move || ed25519::Pair::verify(&signature, &message, &pub_key), - "substrate_ed25519_verify", - ) - } - - /// Push sr25519 signature to verify. - /// - /// Returns false if some of the pushed signatures before already failed the check. - /// (in this case it won't verify anything else) - pub fn push_sr25519( - &mut self, - signature: sr25519::Signature, - pub_key: sr25519::Public, - message: Vec, - ) -> bool { - if self.invalid.load(AtomicOrdering::Relaxed) { - return false - } - self.sr25519_items.push(Sr25519BatchItem { signature, pub_key, message }); - - if self.sr25519_items.len() >= 128 { - let items = std::mem::take(&mut self.sr25519_items); - self.spawn_verification_task( - move || Self::verify_sr25519_batch(items), - "substrate_sr25519_verify", - ) - } else { - true - } - } - - /// Push ecdsa signature to verify. - /// - /// Returns false if some of the pushed signatures before already failed the check - /// (in this case it won't verify anything else) - pub fn push_ecdsa( - &mut self, - signature: ecdsa::Signature, - pub_key: ecdsa::Public, - message: Vec, - ) -> bool { - self.spawn_verification_task( - move || ecdsa::Pair::verify(&signature, &message, &pub_key), - "substrate_ecdsa_verify", - ) - } - - fn verify_sr25519_batch(items: Vec) -> bool { - let messages = items.iter().map(|item| &item.message[..]).collect(); - let signatures = items.iter().map(|item| &item.signature).collect(); - let pub_keys = items.iter().map(|item| &item.pub_key).collect(); - - sr25519::verify_batch(messages, signatures, pub_keys) - } - - /// Verify all previously pushed signatures since last call and return - /// aggregated result. - #[must_use] - pub fn verify_and_clear(&mut self) -> bool { - let pending = std::mem::take(&mut self.pending_tasks); - let started = std::time::Instant::now(); - - log::trace!( - target: "runtime", - "Batch-verification: {} pending tasks, {} sr25519 signatures", - pending.len(), - self.sr25519_items.len(), - ); - - if !Self::verify_sr25519_batch(std::mem::take(&mut self.sr25519_items)) { - return false - } - - if pending.len() > 0 { - let (sender, receiver) = std::sync::mpsc::channel(); - self.scheduler.spawn( - "substrate-batch-verify-join", - None, - async move { - futures::future::join_all(pending).await; - sender.send(()).expect( - "Channel never panics if receiver is live. \ - Receiver is always live until received this data; qed. ", - ); - } - .boxed(), - ); - - if receiver.recv().is_err() { - log::warn!( - target: "runtime", - "Haven't received async result from verification task. Returning false.", - ); - - return false - } - } - - log::trace!( - target: "runtime", - "Finalization of batch verification took {} ms", - started.elapsed().as_millis(), - ); - - !self.invalid.swap(false, AtomicOrdering::Relaxed) - } -} diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 72300eb102177..750b5d5924637 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -40,7 +40,6 @@ use sp_core::{ hexdisplay::HexDisplay, offchain::{OffchainDbExt, OffchainWorkerExt, TransactionPoolExt}, storage::ChildInfo, - traits::TaskExecutorExt, }; #[cfg(feature = "std")] use sp_keystore::KeystoreExt; @@ -75,12 +74,6 @@ use secp256k1::{ #[cfg(feature = "std")] use sp_externalities::{Externalities, ExternalitiesExt}; -#[cfg(feature = "std")] -mod batch_verifier; - -#[cfg(feature = "std")] -use batch_verifier::BatchVerifier; - pub use sp_externalities::MultiRemovalResults; #[cfg(feature = "std")] @@ -801,15 +794,25 @@ pub trait Crypto { /// needs to be called. /// /// Returns `true` when the verification is either successful or batched. + /// + /// NOTE: Is tagged with `register_only` to keep the functions around for backwards + /// compatibility with old runtimes, but it should not be used anymore by new runtimes. + /// The implementation emulates the old behavior, but isn't doing any batch verification + /// anymore. + #[version(1, register_only)] fn ed25519_batch_verify( &mut self, sig: &ed25519::Signature, msg: &[u8], pub_key: &ed25519::Public, ) -> bool { - self.extension::() - .map(|extension| extension.push_ed25519(sig.clone(), *pub_key, msg.to_vec())) - .unwrap_or_else(|| ed25519_verify(sig, msg, pub_key)) + let res = ed25519_verify(sig, msg, pub_key); + + if let Some(ext) = self.extension::() { + ext.0 &= res; + } + + res } /// Verify `sr25519` signature. @@ -828,25 +831,36 @@ pub trait Crypto { /// needs to be called. /// /// Returns `true` when the verification is either successful or batched. + /// + /// NOTE: Is tagged with `register_only` to keep the functions around for backwards + /// compatibility with old runtimes, but it should not be used anymore by new runtimes. + /// The implementation emulates the old behavior, but isn't doing any batch verification + /// anymore. + #[version(1, register_only)] fn sr25519_batch_verify( &mut self, sig: &sr25519::Signature, msg: &[u8], pub_key: &sr25519::Public, ) -> bool { - self.extension::() - .map(|extension| extension.push_sr25519(sig.clone(), *pub_key, msg.to_vec())) - .unwrap_or_else(|| sr25519_verify(sig, msg, pub_key)) + let res = sr25519_verify(sig, msg, pub_key); + + if let Some(ext) = self.extension::() { + ext.0 &= res; + } + + res } /// Start verification extension. + /// + /// NOTE: Is tagged with `register_only` to keep the functions around for backwards + /// compatibility with old runtimes, but it should not be used anymore by new runtimes. + /// The implementation emulates the old behavior, but isn't doing any batch verification + /// anymore. + #[version(1, register_only)] fn start_batch_verify(&mut self) { - let scheduler = self - .extension::() - .expect("No task executor associated with the current context!") - .clone(); - - self.register_extension(VerificationExt(BatchVerifier::new(scheduler))) + self.register_extension(VerificationExtDeprecated(true)) .expect("Failed to register required extension: `VerificationExt`"); } @@ -856,13 +870,19 @@ pub trait Crypto { /// deferred by `sr25519_verify`/`ed25519_verify`. /// /// Will panic if no `VerificationExt` is registered (`start_batch_verify` was not called). + /// + /// NOTE: Is tagged with `register_only` to keep the functions around for backwards + /// compatibility with old runtimes, but it should not be used anymore by new runtimes. + /// The implementation emulates the old behavior, but isn't doing any batch verification + /// anymore. + #[version(1, register_only)] fn finish_batch_verify(&mut self) -> bool { let result = self - .extension::() + .extension::() .expect("`finish_batch_verify` should only be called after `start_batch_verify`") - .verify_and_clear(); + .0; - self.deregister_extension::() + self.deregister_extension::() .expect("No verification extension in current context!"); result @@ -1005,15 +1025,25 @@ pub trait Crypto { /// needs to be called. /// /// Returns `true` when the verification is either successful or batched. + /// + /// NOTE: Is tagged with `register_only` to keep the functions around for backwards + /// compatibility with old runtimes, but it should not be used anymore by new runtimes. + /// The implementation emulates the old behavior, but isn't doing any batch verification + /// anymore. + #[version(1, register_only)] fn ecdsa_batch_verify( &mut self, sig: &ecdsa::Signature, msg: &[u8], pub_key: &ecdsa::Public, ) -> bool { - self.extension::() - .map(|extension| extension.push_ecdsa(sig.clone(), *pub_key, msg.to_vec())) - .unwrap_or_else(|| ecdsa_verify(sig, msg, pub_key)) + let res = ecdsa_verify(sig, msg, pub_key); + + if let Some(ext) = self.extension::() { + ext.0 &= res; + } + + res } /// Verify and recover a SECP256k1 ECDSA signature. @@ -1186,8 +1216,10 @@ pub trait OffchainIndex { #[cfg(feature = "std")] sp_externalities::decl_extension! { - /// Batch verification extension to register/retrieve from the externalities. - pub struct VerificationExt(BatchVerifier); + /// Deprecated verification context. + /// + /// Stores the combined result of all verifications that are done in the same context. + struct VerificationExtDeprecated(bool); } /// Interface that provides functions to access the offchain functionality. @@ -1376,7 +1408,7 @@ pub trait Offchain { /// Read all response headers. /// /// Returns a vector of pairs `(HeaderKey, HeaderValue)`. - /// NOTE response headers have to be read before response body. + /// NOTE: response headers have to be read before response body. fn http_response_headers(&mut self, request_id: HttpRequestId) -> Vec<(Vec, Vec)> { self.extension::() .expect("http_response_headers can be called only in the offchain worker context") @@ -1389,7 +1421,7 @@ pub trait Offchain { /// is reached or server closed the connection. /// If `0` is returned it means that the response has been fully consumed /// and the `request_id` is now invalid. - /// NOTE this implies that response headers must be read before draining the body. + /// NOTE: this implies that response headers must be read before draining the body. /// Passing `None` as a deadline blocks forever. fn http_response_read_body( &mut self, @@ -1684,12 +1716,8 @@ pub type SubstrateHostFunctions = ( #[cfg(test)] mod tests { use super::*; - use sp_core::{ - crypto::UncheckedInto, map, storage::Storage, testing::TaskExecutor, - traits::TaskExecutorExt, - }; + use sp_core::{crypto::UncheckedInto, map, storage::Storage}; use sp_state_machine::BasicExternalities; - use std::any::TypeId; #[test] fn storage_works() { @@ -1781,54 +1809,6 @@ mod tests { }); } - #[test] - fn batch_verify_start_finish_works() { - let mut ext = BasicExternalities::default(); - ext.register_extension(TaskExecutorExt::new(TaskExecutor::new())); - - ext.execute_with(|| { - crypto::start_batch_verify(); - }); - - assert!(ext.extensions().get_mut(TypeId::of::()).is_some()); - - ext.execute_with(|| { - assert!(crypto::finish_batch_verify()); - }); - - assert!(ext.extensions().get_mut(TypeId::of::()).is_none()); - } - - #[test] - fn long_sr25519_batching() { - let mut ext = BasicExternalities::default(); - ext.register_extension(TaskExecutorExt::new(TaskExecutor::new())); - ext.execute_with(|| { - let pair = sr25519::Pair::generate_with_phrase(None).0; - let pair_unused = sr25519::Pair::generate_with_phrase(None).0; - crypto::start_batch_verify(); - for it in 0..70 { - let msg = format!("Schnorrkel {}!", it); - let signature = pair.sign(msg.as_bytes()); - crypto::sr25519_batch_verify(&signature, msg.as_bytes(), &pair.public()); - } - - // push invalid - let msg = b"asdf!"; - let signature = pair.sign(msg); - crypto::sr25519_batch_verify(&signature, msg, &pair_unused.public()); - assert!(!crypto::finish_batch_verify()); - - crypto::start_batch_verify(); - for it in 0..70 { - let msg = format!("Schnorrkel {}!", it); - let signature = pair.sign(msg.as_bytes()); - crypto::sr25519_batch_verify(&signature, msg.as_bytes(), &pair.public()); - } - assert!(crypto::finish_batch_verify()); - }); - } - fn zero_ed_pub() -> ed25519::Public { [0u8; 32].unchecked_into() } @@ -1837,90 +1817,6 @@ mod tests { ed25519::Signature::from_raw([0u8; 64]) } - fn zero_sr_pub() -> sr25519::Public { - [0u8; 32].unchecked_into() - } - - fn zero_sr_sig() -> sr25519::Signature { - sr25519::Signature::from_raw([0u8; 64]) - } - - #[test] - fn batching_works() { - let mut ext = BasicExternalities::default(); - ext.register_extension(TaskExecutorExt::new(TaskExecutor::new())); - - ext.execute_with(|| { - // valid ed25519 signature - crypto::start_batch_verify(); - crypto::ed25519_batch_verify(&zero_ed_sig(), &Vec::new(), &zero_ed_pub()); - assert!(crypto::finish_batch_verify()); - - // 2 valid ed25519 signatures - crypto::start_batch_verify(); - - let pair = ed25519::Pair::generate_with_phrase(None).0; - let msg = b"Important message"; - let signature = pair.sign(msg); - crypto::ed25519_batch_verify(&signature, msg, &pair.public()); - - let pair = ed25519::Pair::generate_with_phrase(None).0; - let msg = b"Even more important message"; - let signature = pair.sign(msg); - crypto::ed25519_batch_verify(&signature, msg, &pair.public()); - - assert!(crypto::finish_batch_verify()); - - // 1 valid, 1 invalid ed25519 signature - crypto::start_batch_verify(); - - let pair1 = ed25519::Pair::generate_with_phrase(None).0; - let pair2 = ed25519::Pair::generate_with_phrase(None).0; - let msg = b"Important message"; - let signature = pair1.sign(msg); - - crypto::ed25519_batch_verify(&zero_ed_sig(), &Vec::new(), &zero_ed_pub()); - crypto::ed25519_batch_verify(&signature, msg, &pair1.public()); - crypto::ed25519_batch_verify(&signature, msg, &pair2.public()); - - assert!(!crypto::finish_batch_verify()); - - // 1 valid ed25519, 2 valid sr25519 - crypto::start_batch_verify(); - - let pair = ed25519::Pair::generate_with_phrase(None).0; - let msg = b"Ed25519 batching"; - let signature = pair.sign(msg); - crypto::ed25519_batch_verify(&signature, msg, &pair.public()); - - let pair = sr25519::Pair::generate_with_phrase(None).0; - let msg = b"Schnorrkel rules"; - let signature = pair.sign(msg); - crypto::sr25519_batch_verify(&signature, msg, &pair.public()); - - let pair = sr25519::Pair::generate_with_phrase(None).0; - let msg = b"Schnorrkel batches!"; - let signature = pair.sign(msg); - crypto::sr25519_batch_verify(&signature, msg, &pair.public()); - - assert!(crypto::finish_batch_verify()); - - // 1 valid sr25519, 1 invalid sr25519 - crypto::start_batch_verify(); - - let pair1 = sr25519::Pair::generate_with_phrase(None).0; - let pair2 = sr25519::Pair::generate_with_phrase(None).0; - let msg = b"Schnorrkcel!"; - let signature = pair1.sign(msg); - - crypto::sr25519_batch_verify(&signature, msg, &pair1.public()); - crypto::sr25519_batch_verify(&signature, msg, &pair2.public()); - crypto::sr25519_batch_verify(&zero_sr_sig(), &Vec::new(), &zero_sr_pub()); - - assert!(!crypto::finish_batch_verify()); - }); - } - #[test] fn use_dalek_ext_works() { let mut ext = BasicExternalities::default(); diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 622eac3d831af..2d959425e0f8e 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -904,40 +904,6 @@ pub fn print(print: impl traits::Printable) { print.print(); } -/// Batching session. -/// -/// To be used in runtime only. Outside of runtime, just construct -/// `BatchVerifier` directly. -#[must_use = "`verify()` needs to be called to finish batch signature verification!"] -pub struct SignatureBatching(bool); - -impl SignatureBatching { - /// Start new batching session. - pub fn start() -> Self { - sp_io::crypto::start_batch_verify(); - SignatureBatching(false) - } - - /// Verify all signatures submitted during the batching session. - #[must_use] - pub fn verify(mut self) -> bool { - self.0 = true; - sp_io::crypto::finish_batch_verify() - } -} - -impl Drop for SignatureBatching { - fn drop(&mut self) { - // Sanity check. If user forgets to actually call `verify()`. - // - // We should not panic if the current thread is already panicking, - // because Rust otherwise aborts the process. - if !self.0 && !sp_std::thread::panicking() { - panic!("Signature verification has not been called before `SignatureBatching::drop`") - } - } -} - /// Describes on what should happen with a storage transaction. pub enum TransactionOutcome { /// Commit the transaction. @@ -962,7 +928,7 @@ mod tests { use super::*; use codec::{Decode, Encode}; - use sp_core::crypto::{Pair, UncheckedFrom}; + use sp_core::crypto::Pair; use sp_io::TestExternalities; use sp_state_machine::create_proof_check_backend; @@ -1045,36 +1011,6 @@ mod tests { assert!(multi_sig.verify(msg, &multi_signer.into_account())); } - #[test] - #[should_panic(expected = "Signature verification has not been called")] - fn batching_still_finishes_when_not_called_directly() { - let mut ext = sp_state_machine::BasicExternalities::default(); - ext.register_extension(sp_core::traits::TaskExecutorExt::new( - sp_core::testing::TaskExecutor::new(), - )); - - ext.execute_with(|| { - let _batching = SignatureBatching::start(); - let dummy = UncheckedFrom::unchecked_from([1; 32]); - let dummy_sig = UncheckedFrom::unchecked_from([1; 64]); - sp_io::crypto::sr25519_verify(&dummy_sig, &Vec::new(), &dummy); - }); - } - - #[test] - #[should_panic(expected = "Hey, I'm an error")] - fn batching_does_not_panic_while_thread_is_already_panicking() { - let mut ext = sp_state_machine::BasicExternalities::default(); - ext.register_extension(sp_core::traits::TaskExecutorExt::new( - sp_core::testing::TaskExecutor::new(), - )); - - ext.execute_with(|| { - let _batching = SignatureBatching::start(); - panic!("Hey, I'm an error"); - }); - } - #[test] fn execute_and_generate_proof_works() { use codec::Encode; diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 90ee962dafa9e..c68cf4d004529 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -163,7 +163,7 @@ mod execution { use sp_core::{ hexdisplay::HexDisplay, storage::{ChildInfo, ChildType, PrefixedStorageKey}, - traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt, RuntimeCode, SpawnNamed}, + traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt, RuntimeCode}, }; use sp_externalities::Extensions; use std::{ @@ -324,11 +324,9 @@ mod execution { call_data: &'a [u8], mut extensions: Extensions, runtime_code: &'a RuntimeCode, - spawn_handle: impl SpawnNamed + Send + 'static, context: CallContext, ) -> Self { extensions.register(ReadRuntimeVersionExt::new(exec.clone())); - extensions.register(sp_core::traits::TaskExecutorExt::new(spawn_handle)); Self { backend, @@ -511,11 +509,10 @@ mod execution { } /// Prove execution using the given state backend, overlayed changes, and call executor. - pub fn prove_execution( + pub fn prove_execution( backend: &mut B, overlay: &mut OverlayedChanges, exec: &Exec, - spawn_handle: Spawn, method: &str, call_data: &[u8], runtime_code: &RuntimeCode, @@ -525,14 +522,12 @@ mod execution { H: Hasher, H::Out: Ord + 'static + codec::Codec, Exec: CodeExecutor + Clone + 'static, - Spawn: SpawnNamed + Send + 'static, { let trie_backend = backend.as_trie_backend(); - prove_execution_on_trie_backend::<_, _, _, _>( + prove_execution_on_trie_backend::<_, _, _>( trie_backend, overlay, exec, - spawn_handle, method, call_data, runtime_code, @@ -549,11 +544,10 @@ mod execution { /// /// 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. - pub fn prove_execution_on_trie_backend( + pub fn prove_execution_on_trie_backend( trie_backend: &TrieBackend, overlay: &mut OverlayedChanges, exec: &Exec, - spawn_handle: Spawn, method: &str, call_data: &[u8], runtime_code: &RuntimeCode, @@ -564,7 +558,6 @@ mod execution { H: Hasher, H::Out: Ord + 'static + codec::Codec, Exec: CodeExecutor + 'static + Clone, - Spawn: SpawnNamed + Send + 'static, { let proving_backend = TrieBackendBuilder::wrap(trie_backend).with_recorder(Default::default()).build(); @@ -577,7 +570,6 @@ mod execution { call_data, extensions, runtime_code, - spawn_handle, CallContext::Offchain, ) .execute_using_consensus_failure_handler::<_>(always_wasm())?; @@ -590,12 +582,11 @@ mod execution { } /// Check execution proof, generated by `prove_execution` call. - pub fn execution_proof_check( + pub fn execution_proof_check( root: H::Out, proof: StorageProof, overlay: &mut OverlayedChanges, exec: &Exec, - spawn_handle: Spawn, method: &str, call_data: &[u8], runtime_code: &RuntimeCode, @@ -604,14 +595,12 @@ mod execution { H: Hasher + 'static, Exec: CodeExecutor + Clone + 'static, H::Out: Ord + 'static + codec::Codec, - Spawn: SpawnNamed + Send + 'static, { let trie_backend = create_proof_check_backend::(root, proof)?; - execution_proof_check_on_trie_backend::<_, _, _>( + execution_proof_check_on_trie_backend::<_, _>( &trie_backend, overlay, exec, - spawn_handle, method, call_data, runtime_code, @@ -619,11 +608,10 @@ mod execution { } /// Check execution proof on proving backend, generated by `prove_execution` call. - pub fn execution_proof_check_on_trie_backend( + pub fn execution_proof_check_on_trie_backend( trie_backend: &TrieBackend, H>, overlay: &mut OverlayedChanges, exec: &Exec, - spawn_handle: Spawn, method: &str, call_data: &[u8], runtime_code: &RuntimeCode, @@ -632,7 +620,6 @@ mod execution { H: Hasher, H::Out: Ord + 'static + codec::Codec, Exec: CodeExecutor + Clone + 'static, - Spawn: SpawnNamed + Send + 'static, { StateMachine::<_, H, Exec>::new( trie_backend, @@ -642,7 +629,6 @@ mod execution { call_data, Extensions::default(), runtime_code, - spawn_handle, CallContext::Offchain, ) .execute_using_consensus_failure_handler(always_untrusted_wasm()) @@ -1309,7 +1295,6 @@ mod tests { use sp_core::{ map, storage::{ChildInfo, StateVersion}, - testing::TaskExecutor, traits::{CallContext, CodeExecutor, Externalities, RuntimeCode}, H256, }; @@ -1384,7 +1369,6 @@ mod tests { &[], Default::default(), &wasm_code, - TaskExecutor::new(), CallContext::Offchain, ); @@ -1413,7 +1397,6 @@ mod tests { &[], Default::default(), &wasm_code, - TaskExecutor::new(), CallContext::Offchain, ); @@ -1443,7 +1426,6 @@ mod tests { &[], Default::default(), &wasm_code, - TaskExecutor::new(), CallContext::Offchain, ); @@ -1475,7 +1457,6 @@ mod tests { &mut remote_backend, &mut Default::default(), &executor, - TaskExecutor::new(), "test", &[], &RuntimeCode::empty(), @@ -1483,12 +1464,11 @@ mod tests { .unwrap(); // check proof locally - let local_result = execution_proof_check::( + let local_result = execution_proof_check::( remote_root, remote_proof, &mut Default::default(), &executor, - TaskExecutor::new(), "test", &[], &RuntimeCode::empty(), diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index a9f6399a9a1b5..3a0165fe4588d 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -34,8 +34,6 @@ use sp_core::{ well_known_keys::{is_child_storage_key, CODE}, StateVersion, Storage, }, - testing::TaskExecutor, - traits::TaskExecutorExt, }; use sp_externalities::{Extension, ExtensionStore, Extensions}; use sp_trie::StorageProof; @@ -105,9 +103,6 @@ where storage.top.insert(CODE.to_vec(), code.to_vec()); - let mut extensions = Extensions::default(); - extensions.register(TaskExecutorExt::new(TaskExecutor::new())); - let offchain_db = TestPersistentOffchainDB::new(); let backend = (storage, state_version).into(); @@ -115,7 +110,7 @@ where TestExternalities { overlay: OverlayedChanges::default(), offchain_db, - extensions, + extensions: Default::default(), backend, storage_transaction_cache: Default::default(), state_version, diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index a91aa99929e2f..5e3c9f703fea1 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -291,7 +291,6 @@ impl let executor = LocalCallExecutor::new( self.backend.clone(), executor, - Box::new(sp_core::testing::TaskExecutor::new()), Default::default(), ExecutionExtensions::new( self.execution_strategies.clone(), diff --git a/utils/frame/benchmarking-cli/src/pallet/command.rs b/utils/frame/benchmarking-cli/src/pallet/command.rs index 15ebc668ff4fb..5016d65b89beb 100644 --- a/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -238,7 +238,6 @@ impl PalletCmd { &(self.extra).encode(), extensions(), &sp_state_machine::backend::BackendRuntimeCode::new(state).runtime_code()?, - sp_core::testing::TaskExecutor::new(), CallContext::Offchain, ) .execute(strategy.into()) @@ -376,7 +375,6 @@ impl PalletCmd { extensions(), &sp_state_machine::backend::BackendRuntimeCode::new(state) .runtime_code()?, - sp_core::testing::TaskExecutor::new(), CallContext::Offchain, ) .execute(strategy.into()) @@ -417,7 +415,6 @@ impl PalletCmd { extensions(), &sp_state_machine::backend::BackendRuntimeCode::new(state) .runtime_code()?, - sp_core::testing::TaskExecutor::new(), CallContext::Offchain, ) .execute(strategy.into()) @@ -450,7 +447,6 @@ impl PalletCmd { extensions(), &sp_state_machine::backend::BackendRuntimeCode::new(state) .runtime_code()?, - sp_core::testing::TaskExecutor::new(), CallContext::Offchain, ) .execute(strategy.into()) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index e1873f6a04abd..733eab7f5a262 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -377,8 +377,7 @@ use sp_core::{ OffchainDbExt, OffchainWorkerExt, TransactionPoolExt, }, storage::well_known_keys, - testing::TaskExecutor, - traits::{CallContext, ReadRuntimeVersion, TaskExecutorExt}, + traits::{CallContext, ReadRuntimeVersion}, twox_128, H256, }; use sp_externalities::Extensions; @@ -813,7 +812,6 @@ where /// Build all extensions that we typically use. pub(crate) fn full_extensions() -> Extensions { let mut extensions = Extensions::default(); - extensions.register(TaskExecutorExt::new(TaskExecutor::new())); let (offchain, _offchain_state) = TestOffchainExt::new(); let (pool, _pool_state) = TestTransactionPoolExt::new(); let keystore = MemoryKeystore::new(); @@ -875,7 +873,6 @@ pub(crate) fn state_machine_call( data, extensions, &sp_state_machine::backend::BackendRuntimeCode::new(&ext.backend).runtime_code()?, - sp_core::testing::TaskExecutor::new(), CallContext::Offchain, ) .execute(sp_state_machine::ExecutionStrategy::AlwaysWasm) @@ -915,7 +912,6 @@ pub(crate) fn state_machine_call_with_proof