Remove deprecated batch verification (#13799)

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.
This commit is contained in:
Bastian Köcher
2023-04-04 12:02:47 +02:00
committed by GitHub
parent 74bbae6901
commit 846ec8cd01
15 changed files with 76 additions and 530 deletions
+62 -166
View File
@@ -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::<VerificationExt>()
.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::<VerificationExtDeprecated>() {
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::<VerificationExt>()
.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::<VerificationExtDeprecated>() {
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::<TaskExecutorExt>()
.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::<VerificationExt>()
.extension::<VerificationExtDeprecated>()
.expect("`finish_batch_verify` should only be called after `start_batch_verify`")
.verify_and_clear();
.0;
self.deregister_extension::<VerificationExt>()
self.deregister_extension::<VerificationExtDeprecated>()
.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::<VerificationExt>()
.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::<VerificationExtDeprecated>() {
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<u8>, Vec<u8>)> {
self.extension::<OffchainWorkerExt>()
.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::<VerificationExt>()).is_some());
ext.execute_with(|| {
assert!(crypto::finish_batch_verify());
});
assert!(ext.extensions().get_mut(TypeId::of::<VerificationExt>()).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();