Build block without checking signatures (#4916)

* in executive

* in other places

* to UnsafeResult

* move doc comment

* apply suggestions

* allow validity mocking for TestXt

* add test

* augment checkable instead of another trait

* fix im online test

* blockbuilder dihotomy

* review suggestions

* update test

* Update client/block-builder/src/lib.rs

* updae spec_version

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/primitives/runtime/src/generic/mod.rs

@@ -39,6 +39,15 @@ pub use self::digest::{
 use crate::codec::Encode;
 use sp_std::prelude::*;
 
+/// Perform singature check.
+#[derive(PartialEq, Eq, Clone, Copy)]
+pub enum CheckSignature {
+	/// Perform.
+	Yes,
+	/// Don't perform.
+	No,
+}
+
 fn encode_with_vec_prefix<T: Encode, F: Fn(&mut Vec<u8>)>(encoder: F) -> Vec<u8> {
 	let size = ::sp_std::mem::size_of::<T>();
 	let reserve = match size {

substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs

@@ -24,7 +24,8 @@ use crate::{
 		self, Member, MaybeDisplay, SignedExtension, Checkable, Extrinsic, ExtrinsicMetadata,
 		IdentifyAccount,
 	},
-	generic::CheckedExtrinsic, transaction_validity::{TransactionValidityError, InvalidTransaction},
+	generic::{CheckSignature, CheckedExtrinsic}, 
+	transaction_validity::{TransactionValidityError, InvalidTransaction},
 };
 
 const TRANSACTION_VERSION: u8 = 4;
@@ -120,18 +121,26 @@ where
 {
 	type Checked = CheckedExtrinsic<AccountId, Call, Extra>;
 
-	fn check(self, lookup: &Lookup) -> Result<Self::Checked, TransactionValidityError> {
+	fn check(self, check_signature: CheckSignature, 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.verify(payload, &signed)
-				}) {
-					return Err(InvalidTransaction::BadProof.into())
-				}
 
-				let (function, extra, _) = raw_payload.deconstruct();
+				let (function, extra) = if let CheckSignature::No = check_signature {
+					(self.function, extra)
+				} else {
+					let raw_payload = SignedPayload::new(self.function, extra)?;
+
+					if !raw_payload.using_encoded(|payload| {
+						signature.verify(payload, &signed)
+					}) {
+						return Err(InvalidTransaction::BadProof.into())
+					}
+					let (function, extra, _) = raw_payload.deconstruct();
+
+					(function, extra)
+				};
+
 				CheckedExtrinsic {
 					signed: Some((signed, extra)),
 					function,
@@ -322,6 +331,7 @@ mod tests {
 	use sp_io::hashing::blake2_256;
 	use crate::codec::{Encode, Decode};
 	use crate::traits::{SignedExtension, IdentifyAccount, IdentityLookup};
+	use crate::generic::CheckSignature;
 	use serde::{Serialize, Deserialize};
 
 	type TestContext = IdentityLookup<u64>;
@@ -402,7 +412,7 @@ mod tests {
 	fn unsigned_check_should_work() {
 		let ux = Ex::new_unsigned(vec![0u8; 0]);
 		assert!(!ux.is_signed().unwrap_or(false));
-		assert!(<Ex as Checkable<TestContext>>::check(ux, &Default::default()).is_ok());
+		assert!(<Ex as Checkable<TestContext>>::check(ux, CheckSignature::Yes, &Default::default()).is_ok());
 	}
 
 	#[test]
@@ -415,7 +425,7 @@ mod tests {
 		);
 		assert!(ux.is_signed().unwrap_or(false));
 		assert_eq!(
-			<Ex as Checkable<TestContext>>::check(ux, &Default::default()),
+			<Ex as Checkable<TestContext>>::check(ux, CheckSignature::Yes, &Default::default()),
 			Err(InvalidTransaction::BadProof.into()),
 		);
 	}
@@ -430,7 +440,7 @@ mod tests {
 		);
 		assert!(ux.is_signed().unwrap_or(false));
 		assert_eq!(
-			<Ex as Checkable<TestContext>>::check(ux, &Default::default()),
+			<Ex as Checkable<TestContext>>::check(ux, CheckSignature::Yes, &Default::default()),
 			Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] }),
 		);
 	}

substrate/primitives/runtime/src/testing.rs

@@ -25,11 +25,10 @@ use crate::traits::{
 };
 #[allow(deprecated)]
 use crate::traits::ValidateUnsigned;
-use crate::{generic, KeyTypeId, ApplyExtrinsicResult};
+use crate::{generic::{self, CheckSignature}, KeyTypeId, ApplyExtrinsicResult};
 pub use sp_core::{H256, sr25519};
 use sp_core::{crypto::{CryptoType, Dummy, key_types, Public}, U256};
-use crate::transaction_validity::{TransactionValidity, TransactionValidityError};
-
+use crate::transaction_validity::{TransactionValidity, TransactionValidityError, InvalidTransaction};
 /// Authority Id
 #[derive(Default, PartialEq, Eq, Clone, Encode, Decode, Debug, Hash, Serialize, Deserialize, PartialOrd, Ord)]
 pub struct UintAuthorityId(pub u64);
@@ -295,12 +294,69 @@ impl<'a, Xt> Deserialize<'a> for Block<Xt> where Block<Xt>: Decode {
 	}
 }
 
-/// Test transaction, tuple of (sender, call, signed_extra)
-/// with index only used if sender is some.
-///
-/// If sender is some then the transaction is signed otherwise it is unsigned.
+/// Test validity.
 #[derive(PartialEq, Eq, Clone, Encode, Decode)]
-pub struct TestXt<Call, Extra>(pub Option<(u64, Extra)>, pub Call);
+pub enum TestValidity {
+	/// Valid variant that will pass all checks.
+	Valid,
+	/// Variant with invalid signature.
+	///
+	/// Will fail signature check.
+	SignatureInvalid(TransactionValidityError),
+	/// Variant with invalid logic.
+	///
+	/// Will fail all checks.
+	OtherInvalid(TransactionValidityError),
+}
+
+/// Test transaction.
+///
+/// Used to mock actual transaction.
+#[derive(PartialEq, Eq, Clone, Encode, Decode)]
+pub struct TestXt<Call, Extra> {
+	/// Signature with extra.
+	///
+	/// if some, then the transaction is signed. Transaction is unsigned otherwise.
+	pub signature: Option<(u64, Extra)>,
+	/// Validity.
+	///
+	/// Instantiate invalid variant and transaction will fail correpsonding checks.
+	pub validity: TestValidity,
+	/// Call.
+	pub call: Call,
+}
+
+impl<Call, Extra> TestXt<Call, Extra> {
+	/// New signed test `TextXt`.
+	pub fn new_signed(signature: (u64, Extra), call: Call) -> Self {
+		TestXt {
+			signature: Some(signature),
+			validity: TestValidity::Valid,
+			call,
+		}
+	}
+
+	/// New unsigned test `TextXt`.
+	pub fn new_unsigned(call: Call) -> Self {
+		TestXt {
+			signature: None,
+			validity: TestValidity::Valid,
+			call,
+		}
+	}
+
+	/// Build invalid variant of `TestXt`.
+	pub fn invalid(mut self, err: TransactionValidityError) -> Self {
+		self.validity = TestValidity::OtherInvalid(err);
+		self
+	}
+
+	/// Build badly signed variant of `TestXt`.
+	pub fn badly_signed(mut self) -> Self {
+		self.validity = TestValidity::SignatureInvalid(TransactionValidityError::Invalid(InvalidTransaction::BadProof));
+		self
+	}
+ }
 
 // Non-opaque extrinsics always 0.
 parity_util_mem::malloc_size_of_is_0!(any: TestXt<Call, Extra>);
@@ -313,24 +369,39 @@ impl<Call, Extra> Serialize for TestXt<Call, Extra> where TestXt<Call, Extra>: E
 
 impl<Call, Extra> Debug for TestXt<Call, Extra> {
 	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		write!(f, "TestXt({:?}, ...)", self.0.as_ref().map(|x| &x.0))
+		write!(f, "TestXt({:?}, {}, ...)",
+			self.signature.as_ref().map(|x| &x.0),
+			if let TestValidity::Valid = self.validity { "valid" } else { "invalid" }
+		)
 	}
 }
 
 impl<Call: Codec + Sync + Send, Context, Extra> Checkable<Context> for TestXt<Call, Extra> {
 	type Checked = Self;
-	fn check(self, _: &Context) -> Result<Self::Checked, TransactionValidityError> { Ok(self) }
+	fn check(self, signature: CheckSignature, _: &Context) -> Result<Self::Checked, TransactionValidityError> {
+		match self.validity {
+			TestValidity::Valid => Ok(self),
+			TestValidity::SignatureInvalid(e) =>
+				if let CheckSignature::No = signature {
+					Ok(self)
+				} else {
+					Err(e)
+				},
+			TestValidity::OtherInvalid(e)  => Err(e),
+		}
+	 }
 }
+
 impl<Call: Codec + Sync + Send, Extra> traits::Extrinsic for TestXt<Call, Extra> {
 	type Call = Call;
 	type SignaturePayload = (u64, Extra);
 
 	fn is_signed(&self) -> Option<bool> {
-		Some(self.0.is_some())
+		Some(self.signature.is_some())
 	}
 
-	fn new(c: Call, sig: Option<Self::SignaturePayload>) -> Option<Self> {
-		Some(TestXt(sig, c))
+	fn new(call: Call, signature: Option<Self::SignaturePayload>) -> Option<Self> {
+		Some(TestXt { signature, call, validity: TestValidity::Valid })
 	}
 }
 
@@ -344,7 +415,7 @@ impl<Origin, Call, Extra, Info> Applyable for TestXt<Call, Extra> where
 	type Call = Call;
 	type DispatchInfo = Info;
 
-	fn sender(&self) -> Option<&Self::AccountId> { self.0.as_ref().map(|x| &x.0) }
+	fn sender(&self) -> Option<&Self::AccountId> { self.signature.as_ref().map(|x| &x.0) }
 
 	/// Checks to see if this is a valid *transaction*. It returns information on it if so.
 	#[allow(deprecated)] // Allow ValidateUnsigned
@@ -364,14 +435,14 @@ impl<Origin, Call, Extra, Info> Applyable for TestXt<Call, Extra> where
 		info: Self::DispatchInfo,
 		len: usize,
 	) -> ApplyExtrinsicResult {
-		let maybe_who = if let Some((who, extra)) = self.0 {
-			Extra::pre_dispatch(extra, &who, &self.1, info, len)?;
+		let maybe_who = if let Some((who, extra)) = self.signature {
+			Extra::pre_dispatch(extra, &who, &self.call, info, len)?;
 			Some(who)
 		} else {
-			Extra::pre_dispatch_unsigned(&self.1, info, len)?;
+			Extra::pre_dispatch_unsigned(&self.call, info, len)?;
 			None
 		};
 
-		Ok(self.1.dispatch(maybe_who.into()).map_err(Into::into))
+		Ok(self.call.dispatch(maybe_who.into()).map_err(Into::into))
 	}
 }

substrate/primitives/runtime/src/traits.rs

@@ -31,7 +31,7 @@ use crate::codec::{Codec, Encode, Decode};
 use crate::transaction_validity::{
 	ValidTransaction, TransactionValidity, TransactionValidityError, UnknownTransaction,
 };
-use crate::generic::{Digest, DigestItem};
+use crate::generic::{Digest, DigestItem, CheckSignature};
 pub use sp_arithmetic::traits::{
 	AtLeast32Bit, UniqueSaturatedInto, UniqueSaturatedFrom, Saturating, SaturatedConversion,
 	Zero, One, Bounded, CheckedAdd, CheckedSub, CheckedMul, CheckedDiv,
@@ -637,7 +637,7 @@ pub trait Checkable<Context>: Sized {
 	type Checked;
 
 	/// Check self, given an instance of Context.
-	fn check(self, c: &Context) -> Result<Self::Checked, TransactionValidityError>;
+	fn check(self, signature: CheckSignature, c: &Context) -> Result<Self::Checked, TransactionValidityError>;
 }
 
 /// A "checkable" piece of information, used by the standard Substrate Executive in order to
@@ -649,15 +649,15 @@ pub trait BlindCheckable: Sized {
 	type Checked;
 
 	/// Check self.
-	fn check(self) -> Result<Self::Checked, TransactionValidityError>;
+	fn check(self, signature: CheckSignature) -> Result<Self::Checked, TransactionValidityError>;
 }
 
 // Every `BlindCheckable` is also a `StaticCheckable` for arbitrary `Context`.
 impl<T: BlindCheckable, Context> Checkable<Context> for T {
 	type Checked = <Self as BlindCheckable>::Checked;
 
-	fn check(self, _c: &Context) -> Result<Self::Checked, TransactionValidityError> {
-		BlindCheckable::check(self)
+	fn check(self, signature: CheckSignature, _c: &Context) -> Result<Self::Checked, TransactionValidityError> {
+		BlindCheckable::check(self, signature)
 	}
 }