Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1d9df92
Enable signature verification in background task
librelois Nov 23, 2021
9a73a44
optimize : spawn all background checks before extrinsics execution
librelois Nov 29, 2021
77a767e
Merge branch 'master' into elois-background-sig-verif
librelois Dec 16, 2021
1ded1ba
Merge branch 'master' into elois-background-sig-verif
librelois Feb 28, 2022
9b974d8
fix warning: fn execute_extrinsics_with_book_keeping not used
librelois Feb 28, 2022
37fcf55
Merge branch 'master' into elois-background-sig-verif
librelois Apr 6, 2022
a222e60
fmt
librelois Apr 6, 2022
80baeb1
refactor in a more generic way
librelois Apr 13, 2022
4a02772
add ci job test-frame-executive-features-compile-to-wasm
librelois Apr 13, 2022
9409c4c
apply review suggestions
librelois May 12, 2022
a4765f4
apply review suggestions 2
librelois May 12, 2022
f3f30d0
Merge branch 'master' into elois-background-sig-verif
librelois May 12, 2022
787db84
Merge branch 'master' into elois-background-sig-verif
librelois Jun 2, 2022
0c4b002
rename SignatureBatching -> BackgroundVerifyContext
librelois Jun 2, 2022
f3557c4
Merge branch 'master' into elois-background-sig-verif
librelois Jul 1, 2022
1bde334
fix try runtime build
librelois Jul 21, 2022
f5a85b0
Merge branch 'master' into elois-background-sig-verif
librelois Jul 21, 2022
5727533
fix tests
librelois Jul 21, 2022
625ce06
fmt
librelois Jul 21, 2022
df3636e
fix warnings
librelois Jul 21, 2022
38a5d82
Merge branch 'master' into elois-background-sig-verif
librelois Aug 25, 2022
6062fa1
Merge branch 'master' into elois-background-sig-verif
librelois Sep 19, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion frame/executive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ sp-inherents = { version = "4.0.0-dev", path = "../../primitives/inherents" }

[features]
default = ["std"]
with-tracing = ["sp-tracing/with-tracing"]

# Enable signature verification in background task
background-sig-check = []
Comment thread
librelois marked this conversation as resolved.
Outdated
std = [
"codec/std",
"scale-info/std",
Expand All @@ -48,3 +50,4 @@ std = [
"sp-std/std",
]
try-runtime = ["frame-support/try-runtime"]
with-tracing = ["sp-tracing/with-tracing"]
106 changes: 102 additions & 4 deletions frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,22 @@ use frame_support::{
},
weights::{DispatchClass, DispatchInfo, GetDispatchInfo},
};
#[cfg(not(feature = "background-sig-check"))]
use sp_runtime::traits::Checkable;
#[cfg(feature = "background-sig-check")]
use sp_runtime::traits::{BackgroundCheckable as Checkable, Checkable as _};
use sp_runtime::{
generic::Digest,
traits::{
self, Applyable, CheckEqual, Checkable, Dispatchable, Header, NumberFor, One, Saturating,
self, Applyable, CheckEqual, Dispatchable, Header, NumberFor, One, Saturating,
ValidateUnsigned, Zero,
},
transaction_validity::{TransactionSource, TransactionValidity},
ApplyExtrinsicResult,
};
use sp_std::{marker::PhantomData, prelude::*};

pub type CheckedOf<E, C> = <E as Checkable<C>>::Checked;
pub type CheckedOf<E, C> = <E as sp_runtime::traits::Checkable<C>>::Checked;
pub type CallOf<E, C> = <CheckedOf<E, C> as Applyable>::Call;
pub type OriginOf<E, C> = <CallOf<E, C> as Dispatchable>::Origin;

Expand Down Expand Up @@ -351,6 +355,7 @@ where
}
}

#[cfg(feature = "background-sig-check")]
Comment thread
librelois marked this conversation as resolved.
Outdated
/// Actually execute all transitions for `block`.
pub fn execute_block(block: Block) {
sp_io::init_tracing();
Expand All @@ -362,12 +367,28 @@ where
// any initial checks
Self::initial_checks(&block);


// check extrinsics in background
let (header, extrinsics) = block.deconstruct();
let signature_batching = sp_runtime::SignatureBatching::start();
let checked_extrinsics = extrinsics
.into_iter()
.map(|extrinsic| {
let encoded = extrinsic.encode();
match extrinsic.background_check(&Default::default()) {
Ok(checked_extrinsic) => (checked_extrinsic, encoded),
Err(e) => {
let err: &'static str = e.into();
panic!("{}", err)
}
}
})
.collect();

// execute extrinsics
let (header, extrinsics) = block.deconstruct();
Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number());
Self::execute_checked_extrinsics_with_book_keeping(checked_extrinsics, *header.number());

// ensure that all background checks have been completed successfully.
if !signature_batching.verify() {
panic!("Signature verification failed.");
}
Expand All @@ -377,7 +398,29 @@ where
}
}

#[cfg(not(feature = "background-sig-check"))]
/// Actually execute all transitions for `block`.
pub fn execute_block(block: Block) {
Comment thread
librelois marked this conversation as resolved.
Outdated
sp_io::init_tracing();
sp_tracing::within_span! {
sp_tracing::info_span!("execute_block", ?block);

Self::initialize_block(block.header());

// any initial checks
Self::initial_checks(&block);

// execute extrinsics
let (header, extrinsics) = block.deconstruct();
Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number());

// any final checks
Self::final_checks(&header);
}
}

/// Execute given extrinsics and take care of post-extrinsics book-keeping.
#[cfg(not(feature = "background-sig-check"))]
fn execute_extrinsics_with_book_keeping(
Comment thread
librelois marked this conversation as resolved.
Outdated
extrinsics: Vec<Block::Extrinsic>,
block_number: NumberFor<Block>,
Expand All @@ -395,6 +438,29 @@ where
Self::idle_and_finalize_hook(block_number);
}

#[cfg(feature = "background-sig-check")]
/// Execute given checked extrinsics and take care of post-extrinsics book-keeping.
fn execute_checked_extrinsics_with_book_keeping(
checked_extrinsics: Vec<(CheckedOf<Block::Extrinsic, Context>, Vec<u8>)>,
Comment thread
librelois marked this conversation as resolved.
Outdated
block_number: NumberFor<Block>,
) {
checked_extrinsics.into_iter().for_each(
Comment thread
librelois marked this conversation as resolved.
Outdated
|(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)
}
},
);

// post-extrinsics book-keeping
<frame_system::Pallet<System>>::note_finished_extrinsics();

Self::idle_and_finalize_hook(block_number);
}

/// Finalize the block - it is up the caller to ensure that all header fields are valid
/// except state-root.
pub fn finalize_block() -> System::Header {
Expand Down Expand Up @@ -465,6 +531,38 @@ where
Ok(r.map(|_| ()).map_err(|e| e.error))
}

#[cfg(feature = "background-sig-check")]
/// Apply checked extrinsic outside of the block execution function.
///
/// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt
/// hashes.
pub fn apply_checked_extrinsic(
Comment thread
librelois marked this conversation as resolved.
Outdated
checked_extrinsic: CheckedOf<Block::Extrinsic, Context>,
unchecked_extrinsic_encoded: Vec<u8>,
) -> ApplyExtrinsicResult {
sp_io::init_tracing();
sp_tracing::enter_span!(sp_tracing::info_span!("apply_checked_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).
<frame_system::Pallet<System>>::note_extrinsic(unchecked_extrinsic_encoded);

// AUDIT: Under no circumstances may this function panic from here onwards.

// Decode parameters and dispatch
let dispatch_info = checked_extrinsic.get_dispatch_info();
let r =
Applyable::apply::<UnsignedValidator>(checked_extrinsic, &dispatch_info, encoded_len)?;

<frame_system::Pallet<System>>::note_applied_extrinsic(&r, dispatch_info);

Ok(r.map(|_| ()).map_err(|e| e.error))
}

fn final_checks(header: &System::Header) {
sp_tracing::enter_span!(sp_tracing::Level::TRACE, "final_checks");
// remove temporaries
Expand Down
34 changes: 32 additions & 2 deletions primitives/runtime/src/generic/unchecked_extrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -163,6 +163,36 @@ where
}
}

impl<Address, AccountId, Call, Signature, Extra, Lookup> BackgroundCheckable<Lookup>
for UncheckedExtrinsic<Address, Call, Signature, Extra>
where
Address: Member + MaybeDisplay,
Call: Encode + Member,
Signature: Member + traits::BackgroundVerify,
<Signature as traits::Verify>::Signer: IdentifyAccount<AccountId = AccountId>,
Extra: SignedExtension<AccountId = AccountId>,
AccountId: Member + MaybeDisplay,
Lookup: traits::Lookup<Source = Address, Target = AccountId>,
{
fn background_check(self, lookup: &Lookup) -> Result<Self::Checked, TransactionValidityError> {
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<Address, Call, Signature, Extra> ExtrinsicMetadata
for UncheckedExtrinsic<Address, Call, Signature, Extra>
where
Expand Down
30 changes: 30 additions & 0 deletions primitives/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,36 @@ impl Verify for MultiSignature {
}
}

impl crate::traits::BackgroundVerify for MultiSignature {
fn background_verify<L: Lazy<[u8]>>(&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()) ==
<dyn AsRef<[u8; 32]>>::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))]
Expand Down
63 changes: 63 additions & 0 deletions primitives/runtime/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,57 @@ where
}
}

/// A signature that supports background verification.
Comment thread
librelois marked this conversation as resolved.
Outdated
pub trait BackgroundVerify: Verify {
Comment thread
bkchr marked this conversation as resolved.
/// Register a signature for background verification.
///
/// This requires that background verification is enabled by doing XYZ.
///
/// 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.
///
/// # Warning
///
/// This requires that the background verification is finished by calling finalize_verify to
/// check the result of all submitted signature verifications.

@bkchr bkchr Apr 12, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Register a signature for background verification.
///
/// This requires that background verification is enabled by doing XYZ.
///
/// 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.
///
/// # Warning
///
/// This requires that the background verification is finished by calling finalize_verify to
/// check the result of all submitted signature verifications.
/// 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 [`SignatureBatching`](crate::SignatureBatching) context.

The SignatureBatching should then be renamed.

fn background_verify<L: Lazy<[u8]>>(
&self,
msg: L,
signer: &<Self::Signer as IdentifyAccount>::AccountId,
) -> bool;
}

impl BackgroundVerify for sp_core::ed25519::Signature {
fn background_verify<L: Lazy<[u8]>>(
&self,
mut msg: L,
signer: &<Self::Signer as IdentifyAccount>::AccountId,
) -> bool {
sp_io::crypto::ed25519_batch_verify(self, msg.get(), signer)
}
}

impl BackgroundVerify for sp_core::sr25519::Signature {
fn background_verify<L: Lazy<[u8]>>(
&self,
mut msg: L,
signer: &<Self::Signer as IdentifyAccount>::AccountId,
) -> bool {
sp_io::crypto::sr25519_batch_verify(self, msg.get(), signer)
}
}

impl BackgroundVerify for sp_core::ecdsa::Signature {
fn background_verify<L: Lazy<[u8]>>(
&self,
mut msg: L,
signer: &<Self::Signer as IdentifyAccount>::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;
Expand Down Expand Up @@ -769,6 +820,18 @@ pub trait Checkable<Context>: Sized {
fn check(self, c: &Context) -> Result<Self::Checked, TransactionValidityError>;
}

/// A piece of information "checkable" in a background task, 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 require some additional
Comment thread
librelois marked this conversation as resolved.
Outdated
/// context `Context` in order to be checked.
pub trait BackgroundCheckable<Context>: Checkable<Context> {
/// Check self in a background tas, given an instance of Context.
fn background_check(
Comment on lines +1048 to +1049

@bkchr bkchr Apr 12, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Check self in a background tas, given an instance of Context.
fn background_check(
/// Check `self` using the given `context`.
///
/// Any signature will be checked using [`BackgroundVerify`].
///
/// # Security
///
/// It is required that this is called in a [`SignatureBatching`](crate::SignatureBatching) context.
fn batch_verification_check(

self,
c: &Context,
) -> Result<<Self as Checkable<Context>>::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
Expand Down