diff --git a/substrate/core/client/src/client.rs b/substrate/core/client/src/client.rs index 444344c4fd..a056ec6c7b 100644 --- a/substrate/core/client/src/client.rs +++ b/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 consensus::BlockImport for Client } } -impl CurrentHeight for Client where - B: backend::Backend, - E: CallExecutor, - Block: BlockT, -{ - type BlockNumber = ::Number; - fn current_height(&self) -> Self::BlockNumber { - self.backend.blockchain().info().best_number - } -} - -impl BlockNumberToHash for Client where - B: backend::Backend, - E: CallExecutor, - Block: BlockT, -{ - type BlockNumber = ::Number; - type Hash = Block::Hash; - fn block_number_to_hash(&self, n: Self::BlockNumber) -> Option { - self.block_hash(n).unwrap_or(None) - } -} - - impl BlockchainEvents for Client where E: CallExecutor, @@ -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 diff --git a/substrate/core/finality-grandpa/src/environment.rs b/substrate/core/finality-grandpa/src/environment.rs index e82cc1f421..3090cff386 100644 --- a/substrate/core/finality-grandpa/src/environment.rs +++ b/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, 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::::one(); diff --git a/substrate/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/substrate/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 092af6e6f3..b803a20397 100644 --- a/substrate/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/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 { 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 { Some(n) } - } #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, Encode, Decode)] struct TestSig(u64, Vec); diff --git a/substrate/core/sr-primitives/src/traits.rs b/substrate/core/sr-primitives/src/traits.rs index 2019bbf126..f7d55dda79 100644 --- a/substrate/core/sr-primitives/src/traits.rs +++ b/substrate/core/sr-primitives/src/traits.rs @@ -122,32 +122,6 @@ impl Lookup for IdentityLookup { fn lookup(&self, x: T) -> result::Result { 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; - - /// 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 { /// Make conversion. diff --git a/substrate/node/cli/src/factory_impl.rs b/substrate/node/cli/src/factory_impl.rs index e902b45f35..dffd084c5f 100644 --- a/substrate/node/cli/src/factory_impl.rs +++ b/substrate/node/cli/src/factory_impl.rs @@ -54,9 +54,10 @@ type Number = <::Header as HeaderT>::Number; impl FactoryState { 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 { key: &Self::Secret, destination: &Self::AccountId, amount: &Self::Balance, + genesis_hash: &::Hash, prior_block_hash: &::Hash, ) -> ::Extrinsic { let index = self.extract_index(&sender, prior_block_hash); @@ -147,7 +149,7 @@ impl RuntimeAdapter for FactoryState { (*amount).into() ) ) - }, key, (prior_block_hash.clone(), (), (), ())) + }, key, (genesis_hash.clone(), prior_block_hash.clone(), (), (), ())) } fn inherent_extrinsics(&self) -> InherentData { diff --git a/substrate/node/cli/src/service.rs b/substrate/node/cli/src/service.rs index 6c45f45d00..3dbddd10f6 100644 --- a/substrate/node/cli/src/service.rs +++ b/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 { diff --git a/substrate/node/executor/src/lib.rs b/substrate/node/executor/src/lib.rs index fa3a5b08bd..e9c7f4e1cb 100644 --- a/substrate/node/executor/src/lib.rs +++ b/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) ) } diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index 64b2d950b1..e9ae43be03 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -432,6 +432,7 @@ pub type SignedBlock = generic::SignedBlock; pub type BlockId = generic::BlockId; /// The SignedExtension to the basic transaction logic. pub type SignedExtra = ( + system::CheckGenesis, system::CheckEra, system::CheckNonce, system::CheckWeight, diff --git a/substrate/srml/executive/src/lib.rs b/substrate/srml/executive/src/lib.rs index b1368e8a28..835b167e3d 100644 --- a/substrate/srml/executive/src/lib.rs +++ b/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) ) } diff --git a/substrate/srml/grandpa/src/lib.rs b/substrate/srml/grandpa/src/lib.rs index 6cde95d457..6f1c897e53 100644 --- a/substrate/srml/grandpa/src/lib.rs +++ b/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 Module { pub fn schedule_pause(in_blocks: T::BlockNumber) -> Result { if let StoredState::Live = >::get() { - let scheduled_at = system::ChainContext::::default().current_height(); + let scheduled_at = >::block_number(); >::put(StoredState::PendingPause { delay: in_blocks, scheduled_at, @@ -247,7 +247,7 @@ impl Module { pub fn schedule_resume(in_blocks: T::BlockNumber) -> Result { if let StoredState::Paused = >::get() { - let scheduled_at = system::ChainContext::::default().current_height(); + let scheduled_at = >::block_number(); >::put(StoredState::PendingResume { delay: in_blocks, scheduled_at, @@ -280,7 +280,7 @@ impl Module { forced: Option, ) -> Result { if !>::exists() { - let scheduled_at = system::ChainContext::::default().current_height(); + let scheduled_at = >::block_number(); if let Some(_) = forced { if Self::next_forced().map_or(false, |next| next > scheduled_at) { diff --git a/substrate/srml/staking/src/tests.rs b/substrate/srml/staking/src/tests.rs index b1c638788b..96c8b9d5f2 100644 --- a/substrate/srml/staking/src/tests.rs +++ b/substrate/srml/staking/src/tests.rs @@ -365,8 +365,8 @@ fn rewards_should_work() { >::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); diff --git a/substrate/srml/system/src/lib.rs b/substrate/srml/system/src/lib.rs index 1d73567c77..ac8357f5ca 100644 --- a/substrate/srml/system/src/lib.rs +++ b/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 CheckWeight { /// 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 SignedExtension for CheckEra { } } +/// Nonce check and increment to give replay protection for transactions. +#[derive(Encode, Decode, Clone, Eq, PartialEq)] +pub struct CheckGenesis(rstd::marker::PhantomData); + +#[cfg(feature = "std")] +impl rstd::fmt::Debug for CheckGenesis { + fn fmt(&self, _f: &mut rstd::fmt::Formatter) -> rstd::fmt::Result { + Ok(()) + } +} + +#[cfg(feature = "std")] +impl CheckGenesis { + pub fn new() -> Self { + Self(std::marker::PhantomData) + } +} + +impl SignedExtension for CheckGenesis { + type AccountId = T::AccountId; + type AdditionalSigned = T::Hash; + fn additional_signed(&self) -> Result { + Ok(>::block_hash(T::BlockNumber::zero())) + } +} + pub struct ChainContext(::rstd::marker::PhantomData); impl Default for ChainContext { fn default() -> Self { @@ -1027,21 +1056,6 @@ impl Lookup for ChainContext { } } -impl CurrentHeight for ChainContext { - type BlockNumber = T::BlockNumber; - fn current_height(&self) -> Self::BlockNumber { - >::block_number() - } -} - -impl BlockNumberToHash for ChainContext { - type BlockNumber = T::BlockNumber; - type Hash = T::Hash; - fn block_number_to_hash(&self, n: Self::BlockNumber) -> Option { - Some(>::block_hash(n)) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/substrate/subkey/src/main.rs b/substrate/subkey/src/main.rs index 9ef3586ed0..4a63138bf6 100644 --- a/substrate/subkey/src/main.rs +++ b/substrate/subkey/src/main.rs @@ -92,9 +92,10 @@ fn execute(matches: clap::ArgMatches) where { let extra = |i: Index, f: Balance| { ( + system::CheckGenesis::::new(), system::CheckEra::::from(Era::Immortal), system::CheckNonce::::from(i), - system::CheckWeight::::from(), + system::CheckWeight::::new(), balances::TakeFees::::from(f), ) }; @@ -170,7 +171,11 @@ fn execute(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(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)[..]) diff --git a/substrate/test-utils/transaction-factory/src/complex_mode.rs b/substrate/test-utils/transaction-factory/src/complex_mode.rs index 66e209fc81..25170f8c18 100644 --- a/substrate/test-utils/transaction-factory/src/complex_mode.rs +++ b/substrate/test-utils/transaction-factory/src/complex_mode.rs @@ -54,6 +54,7 @@ use crate::{RuntimeAdapter, create_block}; pub fn next( factory_state: &mut RA, client: &Arc>>, + genesis_hash: ::Hash, prior_block_hash: ::Hash, prior_block_id: BlockId, ) -> Option<::Block> @@ -91,6 +92,7 @@ where &from.1, &to, &amount, + &genesis_hash, &prior_block_hash, ); diff --git a/substrate/test-utils/transaction-factory/src/lib.rs b/substrate/test-utils/transaction-factory/src/lib.rs index 747811433a..11b080f14f 100644 --- a/substrate/test-utils/transaction-factory/src/lib.rs +++ b/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: &::Hash, prior_block_hash: &::Hash, ) -> ::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::::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::(&mut factory_state, &client, best_hash.into(), best_block_id), - _ => - simple_modes::next::(&mut factory_state, &client, best_hash.into(), best_block_id) + Mode::MasterToNToM => complex_mode::next::( + &mut factory_state, + &client, + genesis_hash, + best_hash.into(), + best_block_id, + ), + _ => simple_modes::next::( + &mut factory_state, + &client, + genesis_hash, + best_hash.into(), + best_block_id, + ), } { best_hash = block.header().hash(); import_block::(&client, block); diff --git a/substrate/test-utils/transaction-factory/src/simple_modes.rs b/substrate/test-utils/transaction-factory/src/simple_modes.rs index 4ce7b47e6f..0554678fbb 100644 --- a/substrate/test-utils/transaction-factory/src/simple_modes.rs +++ b/substrate/test-utils/transaction-factory/src/simple_modes.rs @@ -49,6 +49,7 @@ use crate::{Mode, RuntimeAdapter, create_block}; pub fn next( factory_state: &mut RA, client: &Arc>>, + genesis_hash: ::Hash, prior_block_hash: ::Hash, prior_block_id: BlockId, ) -> Option<::Block> @@ -82,6 +83,7 @@ where &from.1, &to, &amount, + &genesis_hash, &prior_block_hash, );