diff --git a/frame/executive/Cargo.toml b/frame/executive/Cargo.toml index 1ae22ddbb0260..f8407103e2d0e 100644 --- a/frame/executive/Cargo.toml +++ b/frame/executive/Cargo.toml @@ -37,7 +37,9 @@ sp-version = { version = "5.0.0", path = "../../primitives/version" } [features] default = ["std"] -with-tracing = ["sp-tracing/with-tracing"] + +# Enable signature verification in background task +background-signature-verification = [] std = [ "codec/std", "frame-support/std", @@ -50,3 +52,4 @@ std = [ "sp-tracing/std", ] try-runtime = ["frame-support/try-runtime", "frame-try-runtime" ] +with-tracing = ["sp-tracing/with-tracing"] diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 88cefb17c2895..08fb8e470455e 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -125,18 +125,21 @@ use frame_support::{ }, weights::Weight, }; +#[cfg(not(feature = "background-signature-verification"))] +use sp_runtime::traits::Checkable; +#[cfg(feature = "background-signature-verification")] +use sp_runtime::traits::{BackgroundCheckable as Checkable, Checkable as _}; use sp_runtime::{ generic::Digest, traits::{ - self, Applyable, CheckEqual, Checkable, Dispatchable, Header, NumberFor, One, - ValidateUnsigned, Zero, + self, Applyable, CheckEqual, Dispatchable, Header, NumberFor, One, ValidateUnsigned, Zero, }, transaction_validity::{TransactionSource, TransactionValidity}, ApplyExtrinsicResult, }; use sp_std::{marker::PhantomData, prelude::*}; -pub type CheckedOf = >::Checked; +pub type CheckedOf = >::Checked; pub type CallOf = as Applyable>::Call; pub type OriginOf = as Dispatchable>::Origin; @@ -241,9 +244,19 @@ where Self::initialize_block(block.header()); Self::initial_checks(&block); + #[cfg(feature = "background-signature-verification")] + let signature_batching = sp_runtime::BackgroundVerifyContext::start(); + let (header, extrinsics) = block.deconstruct(); + let checked_extrinsics = Self::check_extrinsics(extrinsics); + + Self::execute_checked_extrinsics_with_book_keeping(checked_extrinsics, *header.number()); - Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number()); + // ensure that all background checks have been completed successfully. + #[cfg(feature = "background-signature-verification")] + if !signature_batching.verify() { + panic!("Signature verification failed."); + } // run the try-state checks of all pallets. >::try_state( @@ -405,6 +418,27 @@ where } } + fn check_extrinsics( + extrinsics: Vec, + ) -> impl Iterator, Vec)> { + extrinsics.into_iter().map(|extrinsic| { + let encoded = extrinsic.encode(); + + #[cfg(feature = "background-signature-verification")] + let checked_extrinsic = extrinsic.background_check(&Default::default()); + #[cfg(not(feature = "background-signature-verification"))] + let checked_extrinsic = extrinsic.check(&Default::default()); + + match checked_extrinsic { + Ok(checked_extrinsic) => (checked_extrinsic, encoded), + Err(e) => { + let err: &'static str = e.into(); + panic!("{}", err) + }, + } + }) + } + /// Actually execute all transitions for `block`. pub fn execute_block(block: Block) { sp_io::init_tracing(); @@ -416,12 +450,18 @@ where // any initial checks Self::initial_checks(&block); - let signature_batching = sp_runtime::SignatureBatching::start(); + #[cfg(feature = "background-signature-verification")] + let signature_batching = sp_runtime::BackgroundVerifyContext::start(); - // execute extrinsics + // check extrinsics in background let (header, extrinsics) = block.deconstruct(); - Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number()); + let checked_extrinsics = Self::check_extrinsics(extrinsics); + // execute extrinsics + Self::execute_checked_extrinsics_with_book_keeping(checked_extrinsics, *header.number()); + + // ensure that all background checks have been completed successfully. + #[cfg(feature = "background-signature-verification")] if !signature_batching.verify() { panic!("Signature verification failed."); } @@ -431,13 +471,15 @@ where } } - /// Execute given extrinsics and take care of post-extrinsics book-keeping. - fn execute_extrinsics_with_book_keeping( - extrinsics: Vec, + /// Execute given checked extrinsics and take care of post-extrinsics book-keeping. + fn execute_checked_extrinsics_with_book_keeping( + checked_extrinsics: impl Iterator, Vec)>, block_number: NumberFor, ) { - extrinsics.into_iter().for_each(|e| { - if let Err(e) = Self::apply_extrinsic(e) { + checked_extrinsics.for_each(|(checked_extrinsic, unchecked_extrinsic_encoded)| { + if let Err(e) = + Self::apply_checked_extrinsic(checked_extrinsic, unchecked_extrinsic_encoded) + { let err: &'static str = e.into(); panic!("{}", err) } @@ -485,25 +527,37 @@ where /// /// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt /// hashes. - pub fn apply_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult { + pub fn apply_extrinsic(unchecked_extrinsic: Block::Extrinsic) -> ApplyExtrinsicResult { sp_io::init_tracing(); - let encoded = uxt.encode(); - let encoded_len = encoded.len(); - sp_tracing::enter_span!(sp_tracing::info_span!("apply_extrinsic", - ext=?sp_core::hexdisplay::HexDisplay::from(&encoded))); + let unchecked_extrinsic_encoded = unchecked_extrinsic.encode(); + // Verify that the signature is good. - let xt = uxt.check(&Default::default())?; + let checked_extrinsic = unchecked_extrinsic.check(&Default::default())?; + + Self::apply_checked_extrinsic(checked_extrinsic, unchecked_extrinsic_encoded) + } + + /// Actually apply an extrinsic given its `encoded_len`; this doesn't note its hash. + fn apply_checked_extrinsic( + checked_extrinsic: CheckedOf, + unchecked_extrinsic_encoded: Vec, + ) -> ApplyExtrinsicResult { + sp_tracing::enter_span!(sp_tracing::info_span!("apply_extrinsic", + ext=?sp_core::hexdisplay::HexDisplay::from(&unchecked_extrinsic_encoded))); + + let encoded_len = unchecked_extrinsic_encoded.len(); // We don't need to make sure to `note_extrinsic` only after we know it's going to be // executed to prevent it from leaking in storage since at this point, it will either // execute or panic (and revert storage changes). - >::note_extrinsic(encoded); + >::note_extrinsic(unchecked_extrinsic_encoded); // AUDIT: Under no circumstances may this function panic from here onwards. // Decode parameters and dispatch - let dispatch_info = xt.get_dispatch_info(); - let r = Applyable::apply::(xt, &dispatch_info, encoded_len)?; + let dispatch_info = checked_extrinsic.get_dispatch_info(); + let r = + Applyable::apply::(checked_extrinsic, &dispatch_info, encoded_len)?; >::note_applied_extrinsic(&r, dispatch_info); diff --git a/primitives/runtime/src/generic/unchecked_extrinsic.rs b/primitives/runtime/src/generic/unchecked_extrinsic.rs index fb333abd6ac6e..3e249b275bb93 100644 --- a/primitives/runtime/src/generic/unchecked_extrinsic.rs +++ b/primitives/runtime/src/generic/unchecked_extrinsic.rs @@ -20,8 +20,8 @@ use crate::{ generic::CheckedExtrinsic, traits::{ - self, Checkable, Extrinsic, ExtrinsicMetadata, IdentifyAccount, MaybeDisplay, Member, - SignedExtension, + self, BackgroundCheckable, Checkable, Extrinsic, ExtrinsicMetadata, IdentifyAccount, + MaybeDisplay, Member, SignedExtension, }, transaction_validity::{InvalidTransaction, TransactionValidityError}, OpaqueExtrinsic, @@ -163,6 +163,36 @@ where } } +impl BackgroundCheckable + for UncheckedExtrinsic +where + Address: Member + MaybeDisplay, + Call: Encode + Member, + Signature: Member + traits::BackgroundVerify, + ::Signer: IdentifyAccount, + Extra: SignedExtension, + AccountId: Member + MaybeDisplay, + Lookup: traits::Lookup, +{ + fn background_check(self, lookup: &Lookup) -> Result { + Ok(match self.signature { + Some((signed, signature, extra)) => { + let signed = lookup.lookup(signed)?; + let raw_payload = SignedPayload::new(self.function, extra)?; + if !raw_payload + .using_encoded(|payload| signature.background_verify(payload, &signed)) + { + return Err(InvalidTransaction::BadProof.into()) + } + + let (function, extra, _) = raw_payload.deconstruct(); + CheckedExtrinsic { signed: Some((signed, extra)), function } + }, + None => CheckedExtrinsic { signed: None, function: self.function }, + }) + } +} + impl ExtrinsicMetadata for UncheckedExtrinsic where diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 8017a6ac529a2..547523e1e5827 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -425,6 +425,36 @@ impl Verify for MultiSignature { } } +impl crate::traits::BackgroundVerify for MultiSignature { + fn background_verify>(&self, mut msg: L, signer: &AccountId32) -> bool { + match (self, signer) { + (Self::Ed25519(ref sig), who) => + if let Ok(pubkey) = ed25519::Public::from_slice(who.as_ref()) { + sig.background_verify(msg, &pubkey) + } else { + false + }, + (Self::Sr25519(ref sig), who) => + if let Ok(pubkey) = sr25519::Public::from_slice(who.as_ref()) { + sig.background_verify(msg, &pubkey) + } else { + false + }, + (Self::Ecdsa(ref sig), who) => { + // Unfortunatly, ecdsa signature can't be verified in a background task + // because we don't known the public key. + let m = sp_io::hashing::blake2_256(msg.get()); + match sp_io::crypto::secp256k1_ecdsa_recover_compressed(sig.as_ref(), &m) { + Ok(pubkey) => + &sp_io::hashing::blake2_256(pubkey.as_ref()) == + >::as_ref(who), + _ => false, + } + }, + } + } +} + /// Signature verify that can work with any known signature types.. #[derive(Eq, PartialEq, Clone, Default, Encode, Decode, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] @@ -912,18 +942,18 @@ pub fn print(print: impl traits::Printable) { print.print(); } -/// Batching session. +/// Background verification context. /// /// 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); +#[must_use = "`verify()` needs to be called to finish background verifications!"] +pub struct BackgroundVerifyContext(bool); -impl SignatureBatching { - /// Start new batching session. +impl BackgroundVerifyContext { + /// Start new background verification context. pub fn start() -> Self { sp_io::crypto::start_batch_verify(); - SignatureBatching(false) + BackgroundVerifyContext(false) } /// Verify all signatures submitted during the batching session. @@ -934,14 +964,16 @@ impl SignatureBatching { } } -impl Drop for SignatureBatching { +impl Drop for BackgroundVerifyContext { 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`") + panic!( + "Signature verification has not been called before `BackgroundVerifyContext::drop`" + ) } } } @@ -1062,7 +1094,7 @@ mod tests { )); ext.execute_with(|| { - let _batching = SignatureBatching::start(); + let _batching = BackgroundVerifyContext::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); @@ -1078,7 +1110,7 @@ mod tests { )); ext.execute_with(|| { - let _batching = SignatureBatching::start(); + let _batching = BackgroundVerifyContext::start(); panic!("Hey, I'm an error"); }); } diff --git a/primitives/runtime/src/testing.rs b/primitives/runtime/src/testing.rs index 003fa62c9e088..2effd748041c1 100644 --- a/primitives/runtime/src/testing.rs +++ b/primitives/runtime/src/testing.rs @@ -334,6 +334,17 @@ impl Checkable for TestXt crate::traits::BackgroundCheckable + for TestXt +{ + fn background_check( + self, + _: &Context, + ) -> Result<>::Checked, TransactionValidityError> { + Ok(self) + } +} + impl traits::Extrinsic for TestXt { type Call = Call; type SignaturePayload = (u64, Extra); diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 495dfa66c5462..cb8614e30820f 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -171,6 +171,55 @@ where } } +/// Something that supports background verification. +pub trait BackgroundVerify: Verify { + /// Register a signature for background verification. + /// + /// Returns `true` when the signature was successfully registered for background verification + /// or if background verification is not enabled the signature could be verified successfully + /// immediately. + /// + /// # Security + /// + /// It is required that this is called in a + /// [`BackgroundVerifyContext`](crate::BackgroundVerifyContext) context. + fn background_verify>( + &self, + msg: L, + signer: &::AccountId, + ) -> bool; +} + +impl BackgroundVerify for sp_core::ed25519::Signature { + fn background_verify>( + &self, + mut msg: L, + signer: &::AccountId, + ) -> bool { + sp_io::crypto::ed25519_batch_verify(self, msg.get(), signer) + } +} + +impl BackgroundVerify for sp_core::sr25519::Signature { + fn background_verify>( + &self, + mut msg: L, + signer: &::AccountId, + ) -> bool { + sp_io::crypto::sr25519_batch_verify(self, msg.get(), signer) + } +} + +impl BackgroundVerify for sp_core::ecdsa::Signature { + fn background_verify>( + &self, + mut msg: L, + signer: &::AccountId, + ) -> bool { + sp_io::crypto::ecdsa_batch_verify(self, msg.get(), signer) + } +} + /// An error type that indicates that the origin is invalid. #[derive(Encode, Decode, RuntimeDebug)] pub struct BadOrigin; @@ -991,6 +1040,18 @@ pub trait Checkable: Sized { fn check(self, c: &Context) -> Result; } +/// A "checkable" piece of information, used by the standard Substrate Executive in order to +/// check the validity of a piece of extrinsic information, usually by verifying the signature. Any +/// signature verification should be done using [`BackgroundVerify`]. Implement for pieces of +/// information that require some additional `Context` in order to be checked. +pub trait BackgroundCheckable: Checkable { + /// Check self in a background tas, given an instance of Context. + fn background_check( + self, + c: &Context, + ) -> Result<>::Checked, TransactionValidityError>; +} + /// A "checkable" piece of information, used by the standard Substrate Executive in order to /// check the validity of a piece of extrinsic information, usually by verifying the signature. /// Implement for pieces of information that don't require additional context in order to be diff --git a/scripts/ci/gitlab/pipeline/test.yml b/scripts/ci/gitlab/pipeline/test.yml index bbe38cfb02293..3ed86b649673c 100644 --- a/scripts/ci/gitlab/pipeline/test.yml +++ b/scripts/ci/gitlab/pipeline/test.yml @@ -319,6 +319,21 @@ test-frame-examples-compile-to-wasm: - cargo +nightly build --locked --target=wasm32-unknown-unknown --no-default-features - rusty-cachier cache upload +test-frame-executive-features-compile-to-wasm: + # into one job + stage: test + extends: + - .docker-env + - .test-refs + variables: + # Enable debug assertions since we are running optimized builds for testing + # but still want to have debug assertions. + RUSTFLAGS: "-Cdebug-assertions=y" + RUST_BACKTRACE: 1 + script: + - cd ./frame/executive + - cargo test --features background-signature-verification + test-linux-stable-int: stage: test extends: