Weight Check signed extension. (#3115)

* Weight signed extension.

* Revert a bit + test for check era.

* Update Cargo.toml

* Update node/cli/src/factory_impl.rs

* Update node/executor/src/lib.rs

* Update node/executor/src/lib.rs

core/sr-primitives/src/generic/unchecked_extrinsic.rs

@@ -199,7 +199,7 @@ mod tests {
 	use super::*;
 	use runtime_io::blake2_256;
 	use crate::codec::{Encode, Decode};
-	use crate::traits::SignedExtension;
+	use crate::traits::{SignedExtension, BlockNumberToHash, Lookup, CurrentHeight};
 	use serde::{Serialize, Deserialize};
 
 	struct TestContext;
@@ -237,6 +237,8 @@ mod tests {
 	struct TestExtra;
 	impl SignedExtension for TestExtra {
 		type AccountId = u64;
+		type AdditionalSigned = ();
+		fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) }
 	}
 
 	type Ex = UncheckedExtrinsic<TestAccountId, TestCall, TestSig, TestExtra>;
@@ -254,8 +256,7 @@ mod tests {
 		let ux = Ex::new_signed(
 			vec![0u8; 0],
 			TEST_ACCOUNT,
-			TestSig(TEST_ACCOUNT, (TEST_ACCOUNT, vec![0u8; 0], Era::immortal(), 0u64).encode()),
-			Era::immortal(),
+			TestSig(TEST_ACCOUNT, (vec![0u8; 0], TestExtra).encode()),
 			TestExtra
 		);
 		let encoded = ux.encode();
@@ -267,9 +268,8 @@ mod tests {
 		let ux = Ex::new_signed(
 			vec![0u8; 0],
 			TEST_ACCOUNT,
-			TestSig(TEST_ACCOUNT, (TEST_ACCOUNT, vec![0u8; 257], Era::immortal(), 0u64)
+			TestSig(TEST_ACCOUNT, (vec![0u8; 257], TestExtra)
 				.using_encoded(blake2_256)[..].to_owned()),
-			Era::immortal(),
 			TestExtra
 		);
 		let encoded = ux.encode();
@@ -289,20 +289,18 @@ mod tests {
 			vec![0u8; 0],
 			TEST_ACCOUNT,
 			TestSig(TEST_ACCOUNT, vec![0u8; 0]),
-			Era::immortal(),
 			TestExtra
 		);
 		assert!(ux.is_signed().unwrap_or(false));
 		assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE));
 	}
 
 	#[test]
-	fn immortal_signed_check_should_work() {
+	fn signed_check_should_work() {
 		let ux = Ex::new_signed(
 			vec![0u8; 0],
 			TEST_ACCOUNT,
-			TestSig(TEST_ACCOUNT, (vec![0u8; 0], Era::immortal(), 0u64, TestExtra).encode()),
-			Era::immortal(),
+			TestSig(TEST_ACCOUNT, (vec![0u8; 0], TestExtra).encode()),
 			TestExtra
 		);
 		assert!(ux.is_signed().unwrap_or(false));
@@ -312,63 +310,6 @@ mod tests {
 		);
 	}
 
-	#[test]
-	fn mortal_signed_check_should_work() {
-		let ux = Ex::new_signed(
-			vec![0u8; 0],
-			TEST_ACCOUNT,
-			TestSig(TEST_ACCOUNT, (vec![0u8; 0], Era::mortal(32, 42), 42u64, TestExtra).encode()),
-			Era::mortal(32, 42),
-			TestExtra
-		);
-		assert!(ux.is_signed().unwrap_or(false));
-		assert_eq!(
-			<Ex as Checkable<TestContext>>::check(ux, &TestContext),
-			Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] })
-		);
-	}
-
-	#[test]
-	fn later_mortal_signed_check_should_work() {
-		let ux = Ex::new_signed(
-			vec![0u8; 0],
-			TEST_ACCOUNT,
-			TestSig(TEST_ACCOUNT, (vec![0u8; 0], Era::mortal(32, 11), 11u64, TestExtra).encode()),
-			Era::mortal(32, 11),
-			TestExtra
-		);
-		assert!(ux.is_signed().unwrap_or(false));
-		assert_eq!(
-			<Ex as Checkable<TestContext>>::check(ux, &TestContext),
-			Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] }));
-	}
-
-	#[test]
-	fn too_late_mortal_signed_check_should_fail() {
-		let ux = Ex::new_signed(
-			vec![0u8; 0],
-			TEST_ACCOUNT,
-			TestSig(TEST_ACCOUNT, (TEST_ACCOUNT, vec![0u8; 0], Era::mortal(32, 10), 10u64).encode()),
-			Era::mortal(32, 10),
-			TestExtra
-		);
-		assert!(ux.is_signed().unwrap_or(false));
-		assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE));
-	}
-
-	#[test]
-	fn too_early_mortal_signed_check_should_fail() {
-		let ux = Ex::new_signed(
-			vec![0u8; 0],
-			TEST_ACCOUNT,
-			TestSig(TEST_ACCOUNT, (TEST_ACCOUNT, vec![0u8; 0], Era::mortal(32, 43), 43u64).encode()),
-			Era::mortal(32, 43),
-			TestExtra
-		);
-		assert!(ux.is_signed().unwrap_or(false));
-		assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE));
-	}
-
 	#[test]
 	fn encoding_matches_vec() {
 		let ex = Ex::new_unsigned(vec![0u8; 0]);

core/sr-primitives/src/traits.rs

@@ -848,58 +848,74 @@ pub trait SignedExtension:
 	) -> Result<(), DispatchError> { Self::validate_unsigned(weight).map(|_| ()) }
 }
 
-impl<
-	AccountId,
-	A: SignedExtension<AccountId=AccountId>,
-	B: SignedExtension<AccountId=AccountId>,
-> SignedExtension for (A, B) {
-	type AccountId = AccountId;
-	type AdditionalSigned = (A::AdditionalSigned, B::AdditionalSigned);
-
-	fn additional_signed(&self) -> Result<Self::AdditionalSigned, &'static str> {
-		Ok((self.0.additional_signed()?, self.1.additional_signed()?))
-	}
+macro_rules! tuple_impl_indexed {
+	($first:ident, $($rest:ident,)+ ; $first_index:tt, $($rest_index:tt,)+) => {
+		tuple_impl_indexed!([$first] [$($rest)+] ; [$first_index,] [$($rest_index,)+]);
+	};
+	([$($direct:ident)+] ; [$($index:tt,)+]) => {
+		impl<
+			AccountId,
+			$($direct: SignedExtension<AccountId=AccountId>),+
+		> SignedExtension for ($($direct),+,) {
+			type AccountId = AccountId;
+			type AdditionalSigned = ($($direct::AdditionalSigned,)+);
+			fn additional_signed(&self) -> Result<Self::AdditionalSigned, &'static str> {
+				Ok(( $(self.$index.additional_signed()?,)+ ))
+			}
+			fn validate(
+				&self,
+				who: &Self::AccountId,
+				weight: crate::weights::Weight,
+			) -> Result<ValidTransaction, DispatchError> {
+				let aggregator = vec![$(<$direct as SignedExtension>::validate(&self.$index, who, weight)?),+];
+				Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a)))
+			}
+			fn pre_dispatch(
+				self,
+				who: &Self::AccountId,
+				weight: crate::weights::Weight,
+			) -> Result<(), DispatchError> {
+				$(self.$index.pre_dispatch(who, weight)?;)+
+				Ok(())
+			}
+			fn validate_unsigned(
+				weight: crate::weights::Weight,
+			) -> Result<ValidTransaction, DispatchError> {
+				let aggregator = vec![$($direct::validate_unsigned(weight)?),+];
+				Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a)))
+			}
+			fn pre_dispatch_unsigned(
+				weight: crate::weights::Weight,
+			) -> Result<(), DispatchError> {
+				$($direct::pre_dispatch_unsigned(weight)?;)+
+				Ok(())
+			}
+		}
 
-	fn validate(
-		&self,
-		who: &Self::AccountId,
-		weight: crate::weights::Weight,
-	) -> Result<ValidTransaction, DispatchError> {
-		let a = self.0.validate(who, weight)?;
-		let b = self.1.validate(who, weight)?;
-		Ok(a.combine_with(b))
-	}
-	fn pre_dispatch(
-		self,
-		who: &Self::AccountId,
-		weight: crate::weights::Weight,
-	) -> Result<(), DispatchError> {
-		self.0.pre_dispatch(who, weight)?;
-		self.1.pre_dispatch(who, weight)?;
-		Ok(())
-	}
-	fn validate_unsigned(
-		weight: crate::weights::Weight,
-	) -> Result<ValidTransaction, DispatchError> {
-		let a = A::validate_unsigned(weight)?;
-		let b = B::validate_unsigned(weight)?;
-		Ok(a.combine_with(b))
-	}
-	fn pre_dispatch_unsigned(
-		weight: crate::weights::Weight,
-	) -> Result<(), DispatchError> {
-		A::pre_dispatch_unsigned(weight)?;
-		B::pre_dispatch_unsigned(weight)?;
-		Ok(())
-	}
+	};
+	([$($direct:ident)+] [] ; [$($index:tt,)+] []) => {
+		tuple_impl_indexed!([$($direct)+] ; [$($index,)+]);
+	};
+	(
+		[$($direct:ident)+] [$first:ident $($rest:ident)*]
+		;
+		[$($index:tt,)+] [$first_index:tt, $($rest_index:tt,)*]
+	) => {
+		tuple_impl_indexed!([$($direct)+] ; [$($index,)+]);
+		tuple_impl_indexed!([$($direct)+ $first] [$($rest)*] ; [$($index,)+ $first_index,] [$($rest_index,)*]);
+	};
 }
 
-/// To be used only for testing.
+// TODO: merge this into `tuple_impl` once codec supports `trait Codec` for longer tuple lengths.
+#[allow(non_snake_case)]
+tuple_impl_indexed!(A, B, C, D, E, F, G, H, I, J, ; 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,);
+
+/// Only for base bone testing when you don't care about signed extensions at all.\
 #[cfg(feature = "std")]
 impl SignedExtension for () {
 	type AccountId = u64;
 	type AdditionalSigned = ();
-	fn additional_signed(&self) -> result::Result<(), &'static str> { Ok(()) }
+	fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) }
 }
 
 /// An "executable" piece of information, used by the standard Substrate Executive in order to

core/sr-primitives/src/transaction_validity.rs

@@ -103,7 +103,7 @@ impl ValidTransaction {
 	/// the logic *And* of the propagate flags.
 	pub fn combine_with(mut self, mut other: ValidTransaction) -> Self {
 		ValidTransaction {
-			priority: self.priority + other.priority,
+			priority: self.priority.saturating_add(other.priority),
 			requires: { self.requires.append(&mut other.requires); self.requires },
 			provides: { self.provides.append(&mut other.provides); self.provides },
 			longevity: self.longevity.min(other.longevity),

node-template/runtime/src/lib.rs

@@ -17,7 +17,7 @@ use primitives::bytes;
 use primitives::{ed25519, sr25519, OpaqueMetadata};
 use runtime_primitives::{
 	ApplyResult, transaction_validity::TransactionValidity, generic, create_runtime_str,
-	traits::{self, NumberFor, BlakeTwo256, Block as BlockT, StaticLookup, Verify}
+	traits::{self, NumberFor, BlakeTwo256, Block as BlockT, StaticLookup, Verify}, weights::Weight,
 };
 use client::{
 	block_builder::api::{CheckInherentsResult, InherentData, self as block_builder_api},
@@ -113,6 +113,7 @@ pub fn native_version() -> NativeVersion {
 
 parameter_types! {
 	pub const BlockHashCount: BlockNumber = 250;
+	pub const MaximumBlockWeight: Weight = 4 * 1024 * 1024;
 }
 
 impl system::Trait for Runtime {
@@ -136,6 +137,8 @@ impl system::Trait for Runtime {
 	type Origin = Origin;
 	/// Maximum number of block number to block hash mappings to keep (oldest pruned first).
 	type BlockHashCount = BlockHashCount;
+	/// Maximum weight of each block. With a default weight system of 1byte == 1weight, 4mb is ok.
+	type MaximumBlockWeight = MaximumBlockWeight;
 }
 
 impl aura::Trait for Runtime {
@@ -228,7 +231,7 @@ pub type Block = generic::Block<Header, UncheckedExtrinsic>;
 /// BlockId type as expected by this runtime.
 pub type BlockId = generic::BlockId<Block>;
 /// The SignedExtension to the basic transaction logic.
-pub type SignedExtra = (system::CheckNonce<Runtime>, balances::TakeFees<Runtime>);
+pub type SignedExtra = (system::CheckNonce<Runtime>, system::CheckWeight<Runtime>, balances::TakeFees<Runtime>);
 /// Unchecked extrinsic type as expected by this runtime.
 pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<Address, Call, AccountSignature, SignedExtra>;
 /// Extrinsic type that has already been checked.

node-template/runtime/src/template.rs

@@ -73,6 +73,7 @@ mod tests {
 	use primitives::{H256, Blake2Hasher};
 	use support::{impl_outer_origin, assert_ok, parameter_types};
 	use runtime_primitives::{traits::{BlakeTwo256, IdentityLookup}, testing::Header};
+	use runtime_primitives::weights::Weight;
 
 	impl_outer_origin! {
 		pub enum Origin for Test {}
@@ -85,6 +86,7 @@ mod tests {
 	pub struct Test;
 	parameter_types! {
 		pub const BlockHashCount: u64 = 250;
+		pub const MaximumBlockWeight: Weight = 1024;
 	}
 	impl system::Trait for Test {
 		type Origin = Origin;
@@ -97,6 +99,7 @@ mod tests {
 		type Header = Header;
 		type Event = ();
 		type BlockHashCount = BlockHashCount;
+		type MaximumBlockWeight = MaximumBlockWeight;
 	}
 	impl Trait for Test {
 		type Event = ();

node/cli/src/factory_impl.rs

@@ -51,6 +51,17 @@ pub struct FactoryState<N> {
 
 type Number = <<node_primitives::Block as BlockT>::Header as HeaderT>::Number;
 
+impl<Number> FactoryState<Number> {
+	fn build_extra(index: node_primitives::Index, phase: u64) -> node_runtime::SignedExtra {
+		(
+			system::CheckEra::from(Era::mortal(256, phase)),
+			system::CheckNonce::from(index),
+			system::CheckWeight::from(),
+			balances::TakeFees::from(0)
+		)
+	}
+}
+
 impl RuntimeAdapter for FactoryState<Number> {
 	type AccountId = node_primitives::AccountId;
 	type Balance = node_primitives::Balance;
@@ -127,19 +138,16 @@ impl RuntimeAdapter for FactoryState<Number> {
 	) -> <Self::Block as BlockT>::Extrinsic {
 		let index = self.extract_index(&sender, prior_block_hash);
 		let phase = self.extract_phase(*prior_block_hash);
-		let era = system::CheckEra::from(Era::mortal(256, phase));
-		let check_nonce = system::CheckNonce::from(index);
-		let take_fees = balances::TakeFees::from(0);
 
 		sign::<service::Factory, Self>(CheckedExtrinsic {
-			signed: Some((sender.clone(), (era, (check_nonce, take_fees)))),
+			signed: Some((sender.clone(), Self::build_extra(index, phase))),
 			function: Call::Balances(
 				BalancesCall::transfer(
 					indices::address::Address::Id(destination.clone().into()),
 					(*amount).into()
 				)
 			)
-		}, key, (prior_block_hash.clone(), ((), ())))
+		}, key, (prior_block_hash.clone(), (), (), ()))
 	}
 
 	fn inherent_extrinsics(&self) -> InherentData {

node/cli/src/service.rs

@@ -357,12 +357,14 @@ mod tests {
 			let signer = charlie.clone();
 
 			let function = Call::Balances(BalancesCall::transfer(to.into(), amount));
-			let era = Era::immortal();
+
+			let check_era = system::CheckEra::from(Era::Immortal);
 			let check_nonce = system::CheckNonce::from(index);
+			let check_weight = system::CheckWeight::from();
 			let take_fees = balances::TakeFees::from(0);
-			let extra = (check_nonce, take_fees);
+			let extra = (check_era, check_nonce, check_weight, take_fees);
 
-			let raw_payload = (function, era, genesis_hash, extra.clone());
+			let raw_payload = (function, extra.clone(), genesis_hash);
 			let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 {
 				signer.sign(&blake2_256(payload)[..])
 			} else {
@@ -372,7 +374,6 @@ mod tests {
 				raw_payload.0,
 				from.into(),
 				signature.into(),
-				era,
 				extra,
 			).encode();
 			let v: Vec<u8> = Decode::decode(&mut xt.as_slice()).unwrap();