Revert "Build block without checking signatures (#4916)"

This reverts commit dc92587.

bin/node-template/runtime/src/lib.rs

@@ -297,10 +297,6 @@ impl_runtime_apis! {
 			Executive::apply_extrinsic(extrinsic)
 		}
 
-		fn apply_trusted_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) -> ApplyExtrinsicResult {
-			Executive::apply_trusted_extrinsic(extrinsic)
-		}
-
 		fn finalize_block() -> <Block as BlockT>::Header {
 			Executive::finalize_block()
 		}

bin/node/runtime/src/lib.rs

@@ -83,7 +83,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 230,
+	spec_version: 231,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 };
@@ -705,10 +705,6 @@ impl_runtime_apis! {
 			Executive::apply_extrinsic(extrinsic)
 		}
 
-		fn apply_trusted_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) -> ApplyExtrinsicResult {
-			Executive::apply_trusted_extrinsic(extrinsic)
-		}
-
 		fn finalize_block() -> <Block as BlockT>::Header {
 			Executive::finalize_block()
 		}

client/basic-authorship/src/basic_authorship.rs

@@ -203,7 +203,7 @@ impl<A, B, Block, C> ProposerInner<B, Block, C, A>
 				inherent_data
 			)?
 		{
-			block_builder.push_trusted(extrinsic)?;
+			block_builder.push(extrinsic)?;
 		}
 
 		// proceed with transactions
@@ -226,7 +226,7 @@ impl<A, B, Block, C> ProposerInner<B, Block, C, A>
 			let pending_tx_data = pending_tx.data().clone();
 			let pending_tx_hash = pending_tx.hash().clone();
 			trace!("[{:?}] Pushing to the block.", pending_tx_hash);
-			match sc_block_builder::BlockBuilder::push_trusted(&mut block_builder, pending_tx_data) {
+			match sc_block_builder::BlockBuilder::push(&mut block_builder, pending_tx_data) {
 				Ok(()) => {
 					debug!("[{:?}] Pushed to the block.", pending_tx_hash);
 				}

client/block-builder/src/lib.rs

@@ -145,17 +145,6 @@ where
 	///
 	/// This will ensure the extrinsic can be validly executed (by executing it).
 	pub fn push(&mut self, xt: <Block as BlockT>::Extrinsic) -> Result<(), ApiErrorFor<A, Block>> {
-		self.push_internal(xt, false)
-	}
-
-	/// Push onto the block's list of extrinsics.
-	///
-	/// This will treat incoming extrinsic `xt` as trusted and skip signature check (for signed transactions).
-	pub fn push_trusted(&mut self, xt: <Block as BlockT>::Extrinsic) -> Result<(), ApiErrorFor<A, Block>> {
-		self.push_internal(xt, true)
-	}
-
-	fn push_internal(&mut self, xt: <Block as BlockT>::Extrinsic, skip_signature: bool) -> Result<(), ApiErrorFor<A, Block>> {
 		let block_id = &self.block_id;
 		let extrinsics = &mut self.extrinsics;
 
@@ -172,19 +161,26 @@ where
 					block_id,
 					ExecutionContext::BlockConstruction,
 					xt.clone(),
-				)?
-			} else  {
-				api.apply_extrinsic_with_context(
+				)? {
+					Ok(_) => {
+						extrinsics.push(xt);
+						Ok(())
+					}
+					Err(e) => Err(ApplyExtrinsicFailed::from(e).into()),
+				}
+			})
+		} else {
+			self.api.map_api_result(|api| {
+				match api.apply_extrinsic_with_context(
 					block_id,
 					ExecutionContext::BlockConstruction,
 					xt.clone(),
-				)?
-			};
-
-			match apply_result {
-				Ok(_) => {
-					extrinsics.push(xt);
-					Ok(())
+				)? {
+					Ok(_) => {
+						extrinsics.push(xt);
+						Ok(())
+					}
+					Err(tx_validity) => Err(ApplyExtrinsicFailed::Validity(tx_validity).into()),
 				}
 				Err(tx_validity) => Err(ApplyExtrinsicFailed::Validity(tx_validity).into()),
 			}

client/rpc/src/state/tests.rs

@@ -402,7 +402,7 @@ fn should_return_runtime_version() {
 
 	let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\
 		\"specVersion\":1,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",2],\
-		[\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",5],\
+		[\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",4],\
 		[\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",1],\
 		[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]]}";
 

client/service/src/lib.rs

@@ -656,7 +656,6 @@ mod tests {
 	use futures::executor::block_on;
 	use sp_consensus::SelectChain;
 	use sp_runtime::traits::BlindCheckable;
-	use sp_runtime::generic::CheckSignature;
 	use substrate_test_runtime_client::{prelude::*, runtime::{Extrinsic, Transfer}};
 	use sc_transaction_pool::{BasicPool, FullChainApi};
 
@@ -685,7 +684,7 @@ mod tests {
 
 		// then
 		assert_eq!(transactions.len(), 1);
-		assert!(transactions[0].1.clone().check(CheckSignature::Yes).is_ok());
+		assert!(transactions[0].1.clone().check().is_ok());
 		// this should not panic
 		let _ = transactions[0].1.transfer();
 	}

frame/executive/src/lib.rs

@@ -80,15 +80,13 @@ use frame_support::{
 	weights::{GetDispatchInfo, WeighBlock, DispatchInfo}
 };
 use sp_runtime::{
-	generic::Digest,
-	ApplyExtrinsicResult,
+	generic::Digest, ApplyExtrinsicResult,
 	traits::{
 		self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize,
 		NumberFor, Block as BlockT, OffchainWorker, Dispatchable, Saturating, OnRuntimeUpgrade,
 	},
 	transaction_validity::TransactionValidity,
 };
-use sp_runtime::generic::CheckSignature;
 use sp_runtime::traits::ValidateUnsigned;
 use codec::{Codec, Encode};
 use frame_system::{extrinsics_root, DigestOf};
@@ -263,22 +261,13 @@ where
 	pub fn apply_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult {
 		let encoded = uxt.encode();
 		let encoded_len = encoded.len();
-		Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded), CheckSignature::Yes)
-	}
-
-	/// Apply extrinsic outside of the block execution function.
-	///
-	/// Same as `apply_extrinsic`, but skips signature checks.
-	pub fn apply_trusted_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult {
-		let encoded = uxt.encode();
-		let encoded_len = encoded.len();
-		Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded), CheckSignature::No)
+		Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded))
 	}
 
 	/// Apply an extrinsic inside the block execution function.
 	fn apply_extrinsic_no_note(uxt: Block::Extrinsic) {
 		let l = uxt.encode().len();
-		match Self::apply_extrinsic_with_len(uxt, l, None, CheckSignature::Yes) {
+		match Self::apply_extrinsic_with_len(uxt, l, None) {
 			Ok(_) => (),
 			Err(e) => { let err: &'static str = e.into(); panic!(err) },
 		}
@@ -289,13 +278,9 @@ where
 		uxt: Block::Extrinsic,
 		encoded_len: usize,
 		to_note: Option<Vec<u8>>,
-		check_signature: CheckSignature,
 	) -> ApplyExtrinsicResult {
 		// Verify that the signature is good.
-		let xt = uxt.check(
-			check_signature,
-			&Default::default(),
-		)?;
+		let xt = uxt.check(&Default::default())?;
 
 		// 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
@@ -343,7 +328,7 @@ where
 	/// Changes made to storage should be discarded.
 	pub fn validate_transaction(uxt: Block::Extrinsic) -> TransactionValidity {
 		let encoded_len = uxt.using_encoded(|d| d.len());
-		let xt = uxt.check(CheckSignature::Yes, &Default::default())?;
+		let xt = uxt.check(&Default::default())?;
 
 		let dispatch_info = xt.get_dispatch_info();
 		xt.validate::<UnsignedValidator>(dispatch_info, encoded_len)
@@ -541,8 +526,8 @@ mod tests {
 		)
 	}
 
-	fn sign_extra(who: u64, nonce: u64, fee: u64) -> (u64, SignedExtra) {
-		(who, extra(nonce, fee))
+	fn sign_extra(who: u64, nonce: u64, fee: u64) -> Option<(u64, SignedExtra)> {
+		Some((who, extra(nonce, fee)))
 	}
 
 	#[test]
@@ -551,7 +536,7 @@ mod tests {
 		pallet_balances::GenesisConfig::<Runtime> {
 			balances: vec![(1, 211)],
 		}.assimilate_storage(&mut t).unwrap();
-		let xt = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(2, 69)));
+		let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(2, 69)));
 		let weight = xt.get_dispatch_info().weight as u64;
 		let mut t = sp_io::TestExternalities::new(t);
 		t.execute_with(|| {
@@ -631,7 +616,7 @@ mod tests {
 	fn bad_extrinsic_not_inserted() {
 		let mut t = new_test_ext(1);
 		// bad nonce check!
-		let xt = TestXt::new_signed(sign_extra(1, 30, 0), Call::Balances(BalancesCall::transfer(33, 69)));
+		let xt = sp_runtime::testing::TestXt(sign_extra(1, 30, 0), Call::Balances(BalancesCall::transfer(33, 69)));
 		t.execute_with(|| {
 			Executive::initialize_block(&Header::new(
 				1,
@@ -649,7 +634,7 @@ mod tests {
 	fn block_weight_limit_enforced() {
 		let mut t = new_test_ext(10000);
 		// given: TestXt uses the encoded len as fixed Len:
-		let xt = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0)));
+		let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0)));
 		let encoded = xt.encode();
 		let encoded_len = encoded.len() as Weight;
 		let limit = AvailableBlockRatio::get() * MaximumBlockWeight::get() - 175;
@@ -666,7 +651,7 @@ mod tests {
 			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight(), 175);
 
 			for nonce in 0..=num_to_exhaust_block {
-				let xt = TestXt::new_signed(
+				let xt = sp_runtime::testing::TestXt(
 					sign_extra(1, nonce.into(), 0), Call::Balances(BalancesCall::transfer(33, 0)),
 				);
 				let res = Executive::apply_extrinsic(xt);
@@ -686,9 +671,9 @@ mod tests {
 
 	#[test]
 	fn block_weight_and_size_is_stored_per_tx() {
-		let xt = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0)));
-		let x1 = TestXt::new_signed(sign_extra(1, 1, 0), Call::Balances(BalancesCall::transfer(33, 0)));
-		let x2 = TestXt::new_signed(sign_extra(1, 2, 0), Call::Balances(BalancesCall::transfer(33, 0)));
+		let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0)));
+		let x1 = sp_runtime::testing::TestXt(sign_extra(1, 1, 0), Call::Balances(BalancesCall::transfer(33, 0)));
+		let x2 = sp_runtime::testing::TestXt(sign_extra(1, 2, 0), Call::Balances(BalancesCall::transfer(33, 0)));
 		let len = xt.clone().encode().len() as u32;
 		let mut t = new_test_ext(1);
 		t.execute_with(|| {
@@ -712,7 +697,7 @@ mod tests {
 
 	#[test]
 	fn validate_unsigned() {
-		let xt = TestXt::new_unsigned(Call::Balances(BalancesCall::set_balance(33, 69, 69)));
+		let xt = sp_runtime::testing::TestXt(None, Call::Balances(BalancesCall::set_balance(33, 69, 69)));
 		let mut t = new_test_ext(1);
 
 		t.execute_with(|| {
@@ -721,45 +706,6 @@ mod tests {
 		});
 	}
 
-	#[test]
-	fn unsigned_weight_is_noted_when_applied() {
-		let xt = TestXt::new_unsigned(Call::Balances(BalancesCall::set_balance(33, 69, 69)));
-		let len = xt.clone().encode().len() as u32;
-		let mut t = new_test_ext(1);
-		t.execute_with(|| {
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight(), 0);
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_len(), 0);
-
-			// This is okay -- balances transfer will panic since it requires ensure_signed.
-			assert_eq!(Executive::apply_extrinsic(xt), Ok(Err(DispatchError::BadOrigin)));
-
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight(), len);
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_len(), len);
-		});
-	}
-
-	#[test]
-	fn apply_trusted_skips_signature_check_but_not_others() {
-		let xt1 = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0)))
-			.badly_signed();
-
-		let mut t = new_test_ext(1);
-
-		t.execute_with(|| {
-			assert_eq!(Executive::apply_trusted_extrinsic(xt1), Ok(Ok(())));
-		});
-
-		let xt2 = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0)))
-			.invalid(TransactionValidityError::Invalid(InvalidTransaction::Call));
-
-		t.execute_with(|| {
-			assert_eq!(
-				Executive::apply_trusted_extrinsic(xt2),
-				Err(TransactionValidityError::Invalid(InvalidTransaction::Call))
-			);
-		});
-	}
-
 	#[test]
 	fn can_pay_for_tx_fee_on_full_lock() {
 		let id: LockIdentifier = *b"0       ";
@@ -772,7 +718,7 @@ mod tests {
 					110,
 					lock,
 				);
-				let xt = TestXt::new_signed(
+				let xt = sp_runtime::testing::TestXt(
 					sign_extra(1, 0, 0),
 					Call::System(SystemCall::remark(vec![1u8])),
 				);

frame/im-online/src/tests.rs

@@ -219,7 +219,7 @@ fn should_generate_heartbeats() {
 
 		// check stuff about the transaction.
 		let ex: Extrinsic = Decode::decode(&mut &*transaction).unwrap();
-		let heartbeat = match ex.call {
+		let heartbeat = match ex.1 {
 			crate::mock::Call::ImOnline(crate::Call::heartbeat(h, _)) => h,
 			e => panic!("Unexpected call: {:?}", e),
 		};
@@ -329,7 +329,7 @@ fn should_not_send_a_report_if_already_online() {
 		assert_eq!(pool_state.read().transactions.len(), 0);
 		// check stuff about the transaction.
 		let ex: Extrinsic = Decode::decode(&mut &*transaction).unwrap();
-		let heartbeat = match ex.call {
+		let heartbeat = match ex.1 {
 			crate::mock::Call::ImOnline(crate::Call::heartbeat(h, _)) => h,
 			e => panic!("Unexpected call: {:?}", e),
 		};

primitives/block-builder/src/lib.rs

@@ -24,17 +24,13 @@ use sp_inherents::{InherentData, CheckInherentsResult};
 
 sp_api::decl_runtime_apis! {
 	/// The `BlockBuilder` api trait that provides the required functionality for building a block.
-	#[api_version(5)]
+	#[api_version(4)]
 	pub trait BlockBuilder {
 		/// Apply the given extrinsic.
 		///
 		/// Returns an inclusion outcome which specifies if this extrinsic is included in
 		/// this block or not.
 		fn apply_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) -> ApplyExtrinsicResult;
-		/// Apply the given extrinsic.
-		///
-		/// Same as `apply_extrinsic`, but skips signature verification.
-		fn apply_trusted_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) -> ApplyExtrinsicResult;
 		/// Finish the current block.
 		#[renamed("finalise_block", 3)]
 		fn finalize_block() -> <Block as BlockT>::Header;

primitives/runtime/src/generic/mod.rs

@@ -39,15 +39,6 @@ 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 {