Check the genesis hash in transactions regardless of era. (#3286)

* Check the genesis hash in transactions regardless of era.

* Fix check-fees, too.

* Undo.

* Subkey supports new signing.

* Remove unneeded type param.

* Bump tx version

* Build.

* Another build fix

* Build again

* Cleanup

* Another fix.

* Fix

* Fixes

* 6 second blocks.

* Fixes

* Build fix

* Fix

* Fix.

substrate/core/client/src/client.rs

@@ -35,9 +35,8 @@ use consensus::{
 	SelectChain, self,
 };
 use sr_primitives::traits::{
-	Block as BlockT, Header as HeaderT, Zero, NumberFor, CurrentHeight,
-	BlockNumberToHash, ApiRef, ProvideRuntimeApi,
-	SaturatedConversion, One, DigestFor,
+	Block as BlockT, Header as HeaderT, Zero, NumberFor,
+	ApiRef, ProvideRuntimeApi, SaturatedConversion, One, DigestFor,
 };
 use sr_primitives::generic::DigestItem;
 use sr_primitives::BuildStorage;
@@ -1521,30 +1520,6 @@ impl<B, E, Block, RA> consensus::BlockImport<Block> for Client<B, E, Block, RA>
 	}
 }
 
-impl<B, E, Block, RA> CurrentHeight for Client<B, E, Block, RA> where
-	B: backend::Backend<Block, Blake2Hasher>,
-	E: CallExecutor<Block, Blake2Hasher>,
-	Block: BlockT<Hash=H256>,
-{
-	type BlockNumber = <Block::Header as HeaderT>::Number;
-	fn current_height(&self) -> Self::BlockNumber {
-		self.backend.blockchain().info().best_number
-	}
-}
-
-impl<B, E, Block, RA> BlockNumberToHash for Client<B, E, Block, RA> where
-	B: backend::Backend<Block, Blake2Hasher>,
-	E: CallExecutor<Block, Blake2Hasher>,
-	Block: BlockT<Hash=H256>,
-{
-	type BlockNumber = <Block::Header as HeaderT>::Number;
-	type Hash = Block::Hash;
-	fn block_number_to_hash(&self, n: Self::BlockNumber) -> Option<Self::Hash> {
-		self.block_hash(n).unwrap_or(None)
-	}
-}
-
-
 impl<B, E, Block, RA> BlockchainEvents<Block> for Client<B, E, Block, RA>
 where
 	E: CallExecutor<Block, Blake2Hasher>,
@@ -2698,7 +2673,7 @@ pub(crate) mod tests {
 
 		let current_balance = ||
 			client.runtime_api().balance_of(
-				&BlockId::number(client.current_height()), AccountKeyring::Alice.into()
+				&BlockId::number(client.info().chain.best_number), AccountKeyring::Alice.into()
 			).unwrap();
 
 		// G -> A1 -> A2
@@ -2745,7 +2720,7 @@ pub(crate) mod tests {
 
 		let current_balance = ||
 			client.runtime_api().balance_of(
-				&BlockId::number(client.current_height()), AccountKeyring::Alice.into()
+				&BlockId::number(client.info().chain.best_number), AccountKeyring::Alice.into()
 			).unwrap();
 
 		// G -> A1

substrate/core/finality-grandpa/src/environment.rs

@@ -35,7 +35,7 @@ use grandpa::{
 };
 use sr_primitives::generic::BlockId;
 use sr_primitives::traits::{
-	Block as BlockT, Header as HeaderT, NumberFor, One, Zero, BlockNumberToHash,
+	Block as BlockT, Header as HeaderT, NumberFor, One, Zero,
 };
 use primitives::{Blake2Hasher, ed25519, H256, Pair};
 use substrate_telemetry::{telemetry, CONSENSUS_INFO};
@@ -962,10 +962,10 @@ pub(crate) fn canonical_at_height<B, E, Block: BlockT<Hash=H256>, RA>(
 		if base_is_canonical {
 			return Ok(Some(base.0));
 		} else {
-			return Ok(client.block_number_to_hash(height));
+			return Ok(client.block_hash(height).unwrap_or(None));
 		}
 	} else if base_is_canonical {
-		return Ok(client.block_number_to_hash(height));
+		return Ok(client.block_hash(height).unwrap_or(None));
 	}
 
 	let one = NumberFor::<Block>::one();

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

@@ -25,7 +25,7 @@ use crate::codec::{Decode, Encode, Input};
 use crate::traits::{self, Member, MaybeDisplay, SignedExtension, Checkable, Extrinsic};
 use super::CheckedExtrinsic;
 
-const TRANSACTION_VERSION: u8 = 2;
+const TRANSACTION_VERSION: u8 = 3;
 
 /// A extrinsic right from the external world. This is unchecked and so
 /// can contain a signature.
@@ -205,7 +205,7 @@ mod tests {
 	use super::*;
 	use runtime_io::blake2_256;
 	use crate::codec::{Encode, Decode};
-	use crate::traits::{SignedExtension, BlockNumberToHash, Lookup, CurrentHeight};
+	use crate::traits::{SignedExtension, Lookup};
 	use serde::{Serialize, Deserialize};
 
 	struct TestContext;
@@ -214,15 +214,6 @@ mod tests {
 		type Target = u64;
 		fn lookup(&self, s: u64) -> Result<u64, &'static str> { Ok(s) }
 	}
-	impl CurrentHeight for TestContext {
-		type BlockNumber = u64;
-		fn current_height(&self) -> u64 { 42 }
-	}
-	impl BlockNumberToHash for TestContext {
-		type BlockNumber = u64;
-		type Hash = u64;
-		fn block_number_to_hash(&self, n: u64) -> Option<u64> { Some(n) }
-	}
 
 	#[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, Encode, Decode)]
 	struct TestSig(u64, Vec<u8>);

substrate/core/sr-primitives/src/traits.rs

@@ -122,32 +122,6 @@ impl<T> Lookup for IdentityLookup<T> {
 	fn lookup(&self, x: T) -> result::Result<T, &'static str> { Ok(x) }
 }
 
-/// Get the "current" block number.
-pub trait CurrentHeight {
-	/// The type of the block number.
-	type BlockNumber;
-
-	/// Return the current block number. Not allowed to fail.
-	fn current_height(&self) -> Self::BlockNumber;
-}
-
-/// Translate a block number into a hash.
-pub trait BlockNumberToHash {
-	/// The type of the block number.
-	type BlockNumber: Zero;
-
-	/// The type of the hash.
-	type Hash: Encode;
-
-	/// Get the hash for a given block number, or `None` if unknown.
-	fn block_number_to_hash(&self, n: Self::BlockNumber) -> Option<Self::Hash>;
-
-	/// Get the genesis block hash; this should always be known.
-	fn genesis_hash(&self) -> Self::Hash {
-		self.block_number_to_hash(Zero::zero()).expect("All blockchains must know their genesis block hash; qed")
-	}
-}
-
 /// Extensible conversion trait. Generic over both source and destination types.
 pub trait Convert<A, B> {
 	/// Make conversion.

substrate/node/cli/src/factory_impl.rs

@@ -54,9 +54,10 @@ 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::CheckGenesis::new(),
 			system::CheckEra::from(Era::mortal(256, phase)),
 			system::CheckNonce::from(index),
-			system::CheckWeight::from(),
+			system::CheckWeight::new(),
 			balances::TakeFees::from(0)
 		)
 	}
@@ -134,6 +135,7 @@ impl RuntimeAdapter for FactoryState<Number> {
 		key: &Self::Secret,
 		destination: &Self::AccountId,
 		amount: &Self::Balance,
+		genesis_hash: &<Self::Block as BlockT>::Hash,
 		prior_block_hash: &<Self::Block as BlockT>::Hash,
 	) -> <Self::Block as BlockT>::Extrinsic {
 		let index = self.extract_index(&sender, prior_block_hash);
@@ -147,7 +149,7 @@ impl RuntimeAdapter for FactoryState<Number> {
 					(*amount).into()
 				)
 			)
-		}, key, (prior_block_hash.clone(), (), (), ()))
+		}, key, (genesis_hash.clone(), prior_block_hash.clone(), (), (), ()))
 	}
 
 	fn inherent_extrinsics(&self) -> InherentData {

substrate/node/cli/src/service.rs

@@ -424,11 +424,12 @@ mod tests {
 
 			let function = Call::Balances(BalancesCall::transfer(to.into(), amount));
 
+			let check_genesis = system::CheckGenesis::new();
 			let check_era = system::CheckEra::from(Era::Immortal);
 			let check_nonce = system::CheckNonce::from(index);
-			let check_weight = system::CheckWeight::from();
+			let check_weight = system::CheckWeight::new();
 			let take_fees = balances::TakeFees::from(0);
-			let extra = (check_era, check_nonce, check_weight, take_fees);
+			let extra = (check_genesis, check_era, check_nonce, check_weight, take_fees);
 
 			let raw_payload = (function, extra.clone(), genesis_hash);
 			let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 {

substrate/node/executor/src/lib.rs

@@ -122,7 +122,7 @@ mod tests {
 	fn sign(xt: CheckedExtrinsic) -> UncheckedExtrinsic {
 		match xt.signed {
 			Some((signed, extra)) => {
-				let payload = (xt.function, extra.clone(), GENESIS_HASH);
+				let payload = (xt.function, extra.clone(), GENESIS_HASH, GENESIS_HASH);
 				let key = AccountKeyring::from_public(&signed).unwrap();
 				let signature = payload.using_encoded(|b| {
 					if b.len() > 256 {
@@ -145,9 +145,10 @@ mod tests {
 
 	fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra {
 		(
+			system::CheckGenesis::new(),
 			system::CheckEra::from(Era::mortal(256, 0)),
 			system::CheckNonce::from(nonce),
-			system::CheckWeight::from(),
+			system::CheckWeight::new(),
 			balances::TakeFees::from(extra_fee)
 		)
 	}

substrate/node/runtime/src/lib.rs

@@ -432,6 +432,7 @@ pub type SignedBlock = generic::SignedBlock<Block>;
 pub type BlockId = generic::BlockId<Block>;
 /// The SignedExtension to the basic transaction logic.
 pub type SignedExtra = (
+	system::CheckGenesis<Runtime>,
 	system::CheckEra<Runtime>,
 	system::CheckNonce<Runtime>,
 	system::CheckWeight<Runtime>,

substrate/srml/executive/src/lib.rs

@@ -450,7 +450,7 @@ mod tests {
 		(
 			system::CheckEra::from(Era::Immortal),
 			system::CheckNonce::from(nonce),
-			system::CheckWeight::from(),
+			system::CheckWeight::new(),
 			balances::TakeFees::from(fee)
 		)
 	}

substrate/srml/grandpa/src/lib.rs

@@ -36,7 +36,7 @@ use srml_support::{
 	decl_event, decl_storage, decl_module, dispatch::Result, storage::StorageValue
 };
 use sr_primitives::{
-	generic::{DigestItem, OpaqueDigestItemId}, traits::{CurrentHeight, Zero},
+	generic::{DigestItem, OpaqueDigestItemId}, traits::Zero,
 };
 use fg_primitives::{ScheduledChange, ConsensusLog, GRANDPA_ENGINE_ID};
 pub use fg_primitives::{AuthorityId, AuthorityWeight};
@@ -232,7 +232,7 @@ impl<T: Trait> Module<T> {
 
 	pub fn schedule_pause(in_blocks: T::BlockNumber) -> Result {
 		if let StoredState::Live = <State<T>>::get() {
-			let scheduled_at = system::ChainContext::<T>::default().current_height();
+			let scheduled_at = <system::Module<T>>::block_number();
 			<State<T>>::put(StoredState::PendingPause {
 				delay: in_blocks,
 				scheduled_at,
@@ -247,7 +247,7 @@ impl<T: Trait> Module<T> {
 
 	pub fn schedule_resume(in_blocks: T::BlockNumber) -> Result {
 		if let StoredState::Paused = <State<T>>::get() {
-			let scheduled_at = system::ChainContext::<T>::default().current_height();
+			let scheduled_at = <system::Module<T>>::block_number();
 			<State<T>>::put(StoredState::PendingResume {
 				delay: in_blocks,
 				scheduled_at,
@@ -280,7 +280,7 @@ impl<T: Trait> Module<T> {
 		forced: Option<T::BlockNumber>,
 	) -> Result {
 		if !<PendingChange<T>>::exists() {
-			let scheduled_at = system::ChainContext::<T>::default().current_height();
+			let scheduled_at = <system::Module<T>>::block_number();
 
 			if let Some(_) = forced {
 				if Self::next_forced().map_or(false, |next| next > scheduled_at) {

substrate/srml/staking/src/tests.rs

@@ -365,8 +365,8 @@ fn rewards_should_work() {
 		<Module<Test>>::add_reward_points_to_validator(1001, 10_000);
 
 		// Compute total payout now for whole duration as other parameter won't change
-		let total_payout = current_total_payout_for_duration(9*5);
-		assert!(total_payout > 10); // Test is meaningfull if reward something
+		let total_payout = current_total_payout_for_duration(9 * 5);
+		assert!(total_payout > 10); // Test is meaningful if reward something
 
 		// No reward yet
 		assert_eq!(Balances::total_balance(&2), init_balance_2);

substrate/srml/system/src/lib.rs

@@ -79,12 +79,15 @@ use rstd::map;
 use rstd::marker::PhantomData;
 use sr_primitives::generic::{self, Era};
 use sr_primitives::Perbill;
-use sr_primitives::weights::{Weight, DispatchInfo, DispatchClass, WeightMultiplier, SimpleDispatchInfo};
-use sr_primitives::transaction_validity::{ValidTransaction, TransactionPriority, TransactionLongevity};
+use sr_primitives::weights::{
+	Weight, DispatchInfo, DispatchClass, WeightMultiplier, SimpleDispatchInfo
+};
+use sr_primitives::transaction_validity::{
+	ValidTransaction, TransactionPriority, TransactionLongevity
+};
 use sr_primitives::traits::{self, CheckEqual, SimpleArithmetic, Zero, SignedExtension, Convert,
-	SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash,
-	MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded,
-	Lookup, DispatchError, SaturatedConversion,
+	SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, DispatchError, SaturatedConversion,
+	MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, Lookup,
 };
 use primitives::storage::well_known_keys;
 use srml_support::{
@@ -866,7 +869,7 @@ impl<T: Trait + Send + Sync> CheckWeight<T> {
 
 	/// Utility constructor for tests and client code.
 	#[cfg(feature = "std")]
-	pub fn from() -> Self {
+	pub fn new() -> Self {
 		Self(PhantomData)
 	}
 }
@@ -1012,6 +1015,32 @@ impl<T: Trait + Send + Sync> SignedExtension for CheckEra<T> {
 	}
 }
 
+/// Nonce check and increment to give replay protection for transactions.
+#[derive(Encode, Decode, Clone, Eq, PartialEq)]
+pub struct CheckGenesis<T: Trait + Send + Sync>(rstd::marker::PhantomData<T>);
+
+#[cfg(feature = "std")]
+impl<T: Trait + Send + Sync> rstd::fmt::Debug for CheckGenesis<T> {
+	fn fmt(&self, _f: &mut rstd::fmt::Formatter) -> rstd::fmt::Result {
+		Ok(())
+	}
+}
+
+#[cfg(feature = "std")]
+impl<T: Trait + Send + Sync> CheckGenesis<T> {
+	pub fn new() -> Self {
+		Self(std::marker::PhantomData)
+	}
+}
+
+impl<T: Trait + Send + Sync> SignedExtension for CheckGenesis<T> {
+	type AccountId = T::AccountId;
+	type AdditionalSigned = T::Hash;
+	fn additional_signed(&self) -> Result<Self::AdditionalSigned, &'static str> {
+		Ok(<Module<T>>::block_hash(T::BlockNumber::zero()))
+	}
+}
+
 pub struct ChainContext<T>(::rstd::marker::PhantomData<T>);
 impl<T> Default for ChainContext<T> {
 	fn default() -> Self {
@@ -1027,21 +1056,6 @@ impl<T: Trait> Lookup for ChainContext<T> {
 	}
 }
 
-impl<T: Trait> CurrentHeight for ChainContext<T> {
-	type BlockNumber = T::BlockNumber;
-	fn current_height(&self) -> Self::BlockNumber {
-		<Module<T>>::block_number()
-	}
-}
-
-impl<T: Trait> BlockNumberToHash for ChainContext<T> {
-	type BlockNumber = T::BlockNumber;
-	type Hash = T::Hash;
-	fn block_number_to_hash(&self, n: Self::BlockNumber) -> Option<Self::Hash> {
-		Some(<Module<T>>::block_hash(n))
-	}
-}
-
 #[cfg(test)]
 mod tests {
 	use super::*;

substrate/subkey/src/main.rs

@@ -92,9 +92,10 @@ fn execute<C: Crypto>(matches: clap::ArgMatches) where
 {
 	let extra = |i: Index, f: Balance| {
 		(
+			system::CheckGenesis::<Runtime>::new(),
 			system::CheckEra::<Runtime>::from(Era::Immortal),
 			system::CheckNonce::<Runtime>::from(i),
-			system::CheckWeight::<Runtime>::from(),
+			system::CheckWeight::<Runtime>::new(),
 			balances::TakeFees::<Runtime>::from(f),
 		)
 	};
@@ -170,7 +171,11 @@ fn execute<C: Crypto>(matches: clap::ArgMatches) where
 
 			println!("Using a genesis hash of {}", HexDisplay::from(&genesis_hash.as_ref()));
 
-			let raw_payload = (function, extra(index, 0), genesis_hash);
+			let raw_payload = (
+				function,
+				extra(index, 0),
+				(&genesis_hash, &genesis_hash),
+			);
 			let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 {
 				signer.sign(&blake2_256(payload)[..])
 			} else {
@@ -200,15 +205,20 @@ fn execute<C: Crypto>(matches: clap::ArgMatches) where
 			let function: Call = hex::decode(&call).ok()
 				.and_then(|x| Decode::decode(&mut &x[..])).unwrap();
 
-			let h = matches.value_of("prior-block-hash")
-				.expect("prior-block-hash is required; thus it can't be None; qed");
-			let prior_block_hash: Hash = hex::decode(h).ok()
-				.and_then(|x| Decode::decode(&mut &x[..]))
-				.expect("Invalid prior block hash");
+			let genesis_hash: Hash = match matches.value_of("genesis").unwrap_or("alex") {
+				"elm" => hex!["10c08714a10c7da78f40a60f6f732cf0dba97acfb5e2035445b032386157d5c3"].into(),
+				"alex" => hex!["dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b"].into(),
+				h => hex::decode(h).ok().and_then(|x| Decode::decode(&mut &x[..]))
+					.expect("Invalid genesis hash or unrecognised chain identifier"),
+			};
 
-			let era = Era::immortal();
+			println!("Using a genesis hash of {}", HexDisplay::from(&genesis_hash.as_ref()));
 
-			let raw_payload = (function, era, prior_block_hash, extra(index, 0));
+			let raw_payload = (
+				function,
+				extra(index, 0),
+				(&genesis_hash, &genesis_hash),
+			);
 			let signature = raw_payload.using_encoded(|payload|
 				if payload.len() > 256 {
 					signer.sign(&blake2_256(payload)[..])

substrate/test-utils/transaction-factory/src/complex_mode.rs

@@ -54,6 +54,7 @@ use crate::{RuntimeAdapter, create_block};
 pub fn next<F, RA>(
 	factory_state: &mut RA,
 	client: &Arc<ComponentClient<FullComponents<F>>>,
+	genesis_hash: <RA::Block as BlockT>::Hash,
 	prior_block_hash: <RA::Block as BlockT>::Hash,
 	prior_block_id: BlockId<F::Block>,
 ) -> Option<<F as ServiceFactory>::Block>
@@ -91,6 +92,7 @@ where
 		&from.1,
 		&to,
 		&amount,
+		&genesis_hash,
 		&prior_block_hash,
 	);
 

substrate/test-utils/transaction-factory/src/lib.rs

@@ -77,6 +77,7 @@ pub trait RuntimeAdapter {
 		key: &Self::Secret,
 		destination: &Self::AccountId,
 		amount: &Self::Balance,
+		genesis_hash: &<Self::Block as BlockT>::Hash,
 		prior_block_hash: &<Self::Block as BlockT>::Hash,
 	) -> <Self::Block as BlockT>::Extrinsic;
 
@@ -118,12 +119,24 @@ where
 		select_chain.best_chain().map_err(|e| format!("{:?}", e).into());
 	let mut best_hash = best_header?.hash();
 	let best_block_id = BlockId::<F::Block>::hash(best_hash);
+	let genesis_hash = client.block_hash(Zero::zero())?
+		.expect("Genesis block always exists; qed").into();
 
 	while let Some(block) = match factory_state.mode() {
-		Mode::MasterToNToM =>
-			complex_mode::next::<F, RA>(&mut factory_state, &client, best_hash.into(), best_block_id),
-		_ =>
-			simple_modes::next::<F, RA>(&mut factory_state, &client, best_hash.into(), best_block_id)
+		Mode::MasterToNToM => complex_mode::next::<F, RA>(
+			&mut factory_state,
+			&client,
+			genesis_hash,
+			best_hash.into(),
+			best_block_id,
+		),
+		_ => simple_modes::next::<F, RA>(
+			&mut factory_state,
+			&client,
+			genesis_hash,
+			best_hash.into(),
+			best_block_id,
+		),
 	} {
 		best_hash = block.header().hash();
 		import_block::<F>(&client, block);

substrate/test-utils/transaction-factory/src/simple_modes.rs

@@ -49,6 +49,7 @@ use crate::{Mode, RuntimeAdapter, create_block};
 pub fn next<F, RA>(
 	factory_state: &mut RA,
 	client: &Arc<ComponentClient<FullComponents<F>>>,
+	genesis_hash: <RA::Block as BlockT>::Hash,
 	prior_block_hash: <RA::Block as BlockT>::Hash,
 	prior_block_id: BlockId<F::Block>,
 ) -> Option<<F as ServiceFactory>::Block>
@@ -82,6 +83,7 @@ where
 		&from.1,
 		&to,
 		&amount,
+		&genesis_hash,
 		&prior_block_hash,
 	);