From e26fb6472c3a3b8468b92c2bafce24e3734ac5a2 Mon Sep 17 00:00:00 2001 From: Gav Date: Mon, 15 Oct 2018 10:12:54 +0100 Subject: [PATCH 1/2] Groundwork --- consensus/src/evaluation.rs | 20 +---- consensus/src/lib.rs | 4 +- runtime/src/checked_block.rs | 94 -------------------- runtime/src/lib.rs | 160 +++++++++++------------------------ service/src/chain_spec.rs | 64 +------------- 5 files changed, 61 insertions(+), 281 deletions(-) delete mode 100644 runtime/src/checked_block.rs diff --git a/consensus/src/evaluation.rs b/consensus/src/evaluation.rs index fb860a643..72a90cdb0 100644 --- a/consensus/src/evaluation.rs +++ b/consensus/src/evaluation.rs @@ -19,7 +19,7 @@ use super::MAX_TRANSACTIONS_SIZE; use codec::{Decode, Encode}; -use node_runtime::{Block as GenericBlock, CheckedBlock}; +use node_runtime::{Block as GenericBlock}; use node_primitives::{Hash, BlockNumber, Timestamp}; use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, As}; @@ -30,10 +30,6 @@ error_chain! { description("Proposal provided not a block."), display("Proposal provided not a block."), } - TimestampInFuture { - description("Proposal had timestamp too far in the future."), - display("Proposal had timestamp too far in the future."), - } WrongParentHash(expected: Hash, got: Hash) { description("Proposal had wrong parent hash."), display("Proposal had wrong parent hash. Expected {:?}, got {:?}", expected, got), @@ -56,19 +52,16 @@ error_chain! { /// upon any initial validity checks failing. pub fn evaluate_initial( proposal: &Block, - now: Timestamp, + now: Timestamp, // TODO: should be InherentData parent_hash: &Hash, parent_number: <::Header as HeaderT>::Number, -) -> Result +) -> Result<()> where Hash: PartialEq<<::Header as HeaderT>::Hash>, Hash: Into + Clone, { - const MAX_TIMESTAMP_DRIFT: Timestamp = 60; - let encoded = Encode::encode(proposal); let proposal = GenericBlock::decode(&mut &encoded[..]) - .and_then(|b| CheckedBlock::new(b).ok()) .ok_or_else(|| ErrorKind::BadProposalFormat)?; let transactions_size = proposal.extrinsics.iter().fold(0, |a, tx| { @@ -87,12 +80,7 @@ where bail!(ErrorKind::WrongNumber(parent_number.as_() + 1, proposal.header.number)); } - let block_timestamp = proposal.timestamp(); - - // lenient maximum -- small drifts will just be delayed using a timer. - if block_timestamp > now + MAX_TIMESTAMP_DRIFT { - bail!(ErrorKind::TimestampInFuture) - } + // TODO: Should use Ok(proposal) } diff --git a/consensus/src/lib.rs b/consensus/src/lib.rs index b8bb32e3a..4fbb233ae 100644 --- a/consensus/src/lib.rs +++ b/consensus/src/lib.rs @@ -481,9 +481,9 @@ impl bft::Proposer<::Block> for Proposer where let signature = self.local_key.sign(&payload.encode()).into(); next_index += 1; - let local_id = self.local_key.public().0.into(); + let local_id: AccountId = self.local_key.public().0.into(); let extrinsic = UncheckedExtrinsic { - signature: Some((node_runtime::RawAddress::Id(local_id), signature, payload.0, Era::immortal())), + signature: Some((local_id.into(), signature, payload.0, Era::immortal())), function: payload.1, }; let uxt: <::Block as BlockT>::Extrinsic = Decode::decode(&mut extrinsic.encode().as_slice()).expect("Encoded extrinsic is valid"); diff --git a/runtime/src/checked_block.rs b/runtime/src/checked_block.rs deleted file mode 100644 index bba748dd1..000000000 --- a/runtime/src/checked_block.rs +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright 2018 Parity Technologies (UK) Ltd. -// This file is part of Substrate. - -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see . - -//! Typesafe block interaction. - -use super::{Call, Block, TIMESTAMP_SET_POSITION, NOTE_OFFLINE_POSITION}; -use timestamp::Call as TimestampCall; -use consensus::Call as ConsensusCall; - -/// Provides a type-safe wrapper around a structurally valid block. -pub struct CheckedBlock { - inner: Block, - file_line: Option<(&'static str, u32)>, -} - -impl CheckedBlock { - /// Create a new checked block. Fails if the block is not structurally valid. - pub fn new(block: Block) -> Result { - let has_timestamp = block.extrinsics.get(TIMESTAMP_SET_POSITION as usize).map_or(false, |xt| { - !xt.is_signed() && match xt.function { - Call::Timestamp(TimestampCall::set(_)) => true, - _ => false, - } - }); - - if !has_timestamp { return Err(block) } - - Ok(CheckedBlock { - inner: block, - file_line: None, - }) - } - - // Creates a new checked block, asserting that it is valid. - #[doc(hidden)] - pub fn new_unchecked(block: Block, file: &'static str, line: u32) -> Self { - CheckedBlock { - inner: block, - file_line: Some((file, line)), - } - } - - /// Extract the timestamp from the block. - pub fn timestamp(&self) -> ::node_primitives::Timestamp { - let x = self.inner.extrinsics.get(TIMESTAMP_SET_POSITION as usize).and_then(|xt| match xt.function { - Call::Timestamp(TimestampCall::set(x)) => Some(x), - _ => None - }); - - match x { - Some(x) => x, - None => panic!("Invalid block asserted at {:?}", self.file_line), - } - } - - /// Extract the noted missed proposal validator indices (if any) from the block. - pub fn noted_offline(&self) -> &[u32] { - self.inner.extrinsics.get(NOTE_OFFLINE_POSITION as usize).and_then(|xt| match xt.function { - Call::Consensus(ConsensusCall::note_offline(ref x)) => Some(&x[..]), - _ => None, - }).unwrap_or(&[]) - } - - /// Convert into inner block. - pub fn into_inner(self) -> Block { self.inner } -} - -impl ::std::ops::Deref for CheckedBlock { - type Target = Block; - - fn deref(&self) -> &Block { &self.inner } -} - -/// Assert that a block is structurally valid. May lead to panic in the future -/// in case it isn't. -#[macro_export] -macro_rules! assert_node_block { - ($block: expr) => { - $crate::CheckedBlock::new_unchecked($block, file!(), line!()) - } -} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 5315875e1..60659ea8b 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! The Substrate runtime. This can be compiled with ``#[no_std]`, ready for Wasm. +//! The Substrate runtime. This can be compiled with `#[no_std]`, ready for Wasm. #![cfg_attr(not(feature = "std"), no_std)] // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256. @@ -57,22 +57,15 @@ extern crate srml_treasury as treasury; extern crate sr_version as version; extern crate node_primitives; -#[cfg(feature = "std")] -mod checked_block; - use rstd::prelude::*; -use substrate_primitives::u32_trait::{_2, _4}; use node_primitives::{AccountId, AccountIndex, Balance, BlockNumber, Hash, Index, SessionKey, Signature, InherentData}; use runtime_api::runtime::*; use runtime_primitives::ApplyResult; use runtime_primitives::transaction_validity::TransactionValidity; use runtime_primitives::generic; -use runtime_primitives::traits::{Convert, BlakeTwo256, DigestItem, Block as BlockT}; +use runtime_primitives::traits::{BlakeTwo256, DigestItem, Block as BlockT}; use version::{RuntimeVersion, ApiId}; -use council::{motions as council_motions, voting as council_voting}; #[cfg(feature = "std")] -use council::seats as council_seats; -#[cfg(any(feature = "std", test))] use version::NativeVersion; #[cfg(any(feature = "std", test))] @@ -89,21 +82,26 @@ pub use checked_block::CheckedBlock; const TIMESTAMP_SET_POSITION: u32 = 0; const NOTE_OFFLINE_POSITION: u32 = 1; -const INHERENT: ApiId = *b"inherent"; -const VALIDATX: ApiId = *b"validatx"; +const BLOCK_BUILDER: ApiId = *b"blkbuild"; +const TAGGED_TRANSACTION_QUEUE: ApiId = *b"validatx"; +const METADATA: ApiId = *b"metadata"; /// Runtime version. pub const VERSION: RuntimeVersion = RuntimeVersion { - spec_name: ver_str!("node"), - impl_name: ver_str!("substrate-node"), + spec_name: ver_str!("template-node"), + impl_name: ver_str!("substrate-template-node"), authoring_version: 1, spec_version: 1, impl_version: 0, - apis: apis_vec!([(INHERENT, 1), (VALIDATX, 1)]), + apis: apis_vec!([ + (BLOCK_BUILDER, 1), + (TAGGED_TRANSACTION_QUEUE, 1), + (METADATA, 1) + ]), }; /// Native version. -#[cfg(any(feature = "std", test))] +#[cfg(feature = "std")] pub fn native_version() -> NativeVersion { NativeVersion { runtime_version: VERSION, @@ -124,19 +122,11 @@ impl system::Trait for Runtime { type Log = Log; } -impl balances::Trait for Runtime { - type Balance = Balance; - type AccountIndex = AccountIndex; - type OnFreeBalanceZero = (Staking, Contract); - type EnsureAccountLiquid = Staking; - type Event = Event; -} - impl consensus::Trait for Runtime { const NOTE_OFFLINE_POSITION: u32 = NOTE_OFFLINE_POSITION; type Log = Log; type SessionKey = SessionKey; - type OnOfflineValidator = Staking; + type OnOfflineValidator = (); } impl timestamp::Trait for Runtime { @@ -144,56 +134,15 @@ impl timestamp::Trait for Runtime { type Moment = u64; } -/// Session key conversion. -pub struct SessionKeyConversion; -impl Convert for SessionKeyConversion { - fn convert(a: AccountId) -> SessionKey { - a.0.into() - } -} - -impl session::Trait for Runtime { - type ConvertAccountIdToSessionKey = SessionKeyConversion; - type OnSessionChange = Staking; - type Event = Event; -} - -impl staking::Trait for Runtime { - type OnRewardMinted = Treasury; - type Event = Event; -} - -impl democracy::Trait for Runtime { - type Proposal = Call; - type Event = Event; -} - -impl council::Trait for Runtime { - type Event = Event; -} - -impl council::voting::Trait for Runtime { - type Event = Event; -} - -impl council::motions::Trait for Runtime { - type Origin = Origin; - type Proposal = Call; - type Event = Event; -} - -impl treasury::Trait for Runtime { - type ApproveOrigin = council_motions::EnsureMembers<_4>; - type RejectOrigin = council_motions::EnsureMembers<_2>; - type Event = Event; -} - -impl contract::Trait for Runtime { - type Gas = u64; - type DetermineContractAddress = contract::SimpleAddressDeterminator; +impl balances::Trait for Runtime { + type Balance = Balance; + type AccountIndex = AccountIndex; + type OnFreeBalanceZero = (); + type EnsureAccountLiquid = (); type Event = Event; } +// TODO: v1: should be macro impl DigestItem for Log { type Hash = Hash; type AuthorityId = SessionKey; @@ -219,22 +168,12 @@ construct_runtime!( Consensus: consensus::{Module, Call, Storage, Config, Log(AuthoritiesChange)}, Balances: balances, Timestamp: timestamp::{Module, Call, Storage, Config}, - Session: session, - Staking: staking, - Democracy: democracy, - Council: council::{Module, Call, Storage, Event}, - CouncilVoting: council_voting, - CouncilMotions: council_motions::{Module, Call, Storage, Event, Origin}, - CouncilSeats: council_seats::{Config}, - Treasury: treasury, - Contract: contract::{Module, Call, Config, Event}, } ); -/// The address format for describing accounts. -pub use balances::address::Address as RawAddress; -/// The address format for describing accounts. -pub type Address = balances::Address; +type Context = system::ChainContext; +type Address = AccountId; + /// Block header type as expected by this runtime. pub type Header = generic::Header; /// Block type as expected by this runtime. @@ -246,7 +185,7 @@ pub type UncheckedExtrinsic = generic::UncheckedMortalExtrinsic; /// Executive: handles dispatch to the various modules. -pub type Executive = executive::Executive, Balances, AllModules>; +pub type Executive = executive::Executive; impl_apis! { impl Core for Runtime { @@ -283,6 +222,7 @@ impl_apis! { } fn inherent_extrinsics(data: InherentData) -> Vec { + // TODO: v1: should be automatically gathered. let mut inherent = vec![generic::UncheckedMortalExtrinsic::new_unsigned( Call::Timestamp(TimestampCall::set(data.timestamp)) )]; @@ -296,18 +236,34 @@ impl_apis! { inherent } - fn random_seed() -> ::Hash { - System::random_seed() - } - } + fn check_inherents(block: Block, data: InherentData) -> Result<(), &'static str> { + // TODO: v1: should be automatically gathered + + // Timestamp module... + const MAX_TIMESTAMP_DRIFT: Timestamp = 60; + let xt = block.extrinsics.get(TIMESTAMP_SET_POSITION as usize) + .ok_or("No valid timestamp inherent in block")?; + let t = match (xt.is_signed(), xt.function) { + (false, Call::Timestamp(TimestampCall::set(t))) => t, + _ => bail!("No valid timestamp inherent in block"), + }; + if t > now + MAX_TIMESTAMP_DRIFT { + bail!("Timestamp in future") + } - impl OldTxQueue for Runtime { - fn account_nonce(account: AccountId) -> Index { - System::account_nonce(&account) - } + // Offline indices + let noted_offline = + self.inner.extrinsics.get(NOTE_OFFLINE_POSITION as usize).and_then(|xt| match xt.function { + Call::Consensus(ConsensusCall::note_offline(ref x)) => Some(&x[..]), + _ => None, + }).unwrap_or(&[]); + for i in noted_offline { + if data.offline_indices.iter().find(i).is_none() { + bail!("Online node marked offline") + } + } - fn lookup_address(address: Address) -> Option { - Balances::lookup_address(address) + Ok(()) } } @@ -316,18 +272,4 @@ impl_apis! { Executive::validate_transaction(tx) } } - - impl Miscellaneous for Runtime { - fn validator_count() -> u32 { - Session::validator_count() - } - - fn validators() -> Vec { - Session::validators() - } - - fn timestamp() -> u64 { - Timestamp::get() - } - } } diff --git a/service/src/chain_spec.rs b/service/src/chain_spec.rs index ae04e8290..473f476f5 100644 --- a/service/src/chain_spec.rs +++ b/service/src/chain_spec.rs @@ -18,9 +18,7 @@ use primitives::{AuthorityId, ed25519}; use node_primitives::AccountId; -use node_runtime::{GenesisConfig, ConsensusConfig, CouncilSeatsConfig, CouncilVotingConfig, DemocracyConfig, - SessionConfig, StakingConfig, TimestampConfig, BalancesConfig, TreasuryConfig, - ContractConfig, Permill, Perbill}; +use node_runtime::{GenesisConfig, ConsensusConfig, TimestampConfig, BalancesConfig}; use service::ChainSpec; // Note this is the URL for the telemetry server @@ -33,6 +31,9 @@ fn testnet_genesis(initial_authorities: Vec, endowed_accounts: Vec< authorities: initial_authorities.clone(), }), system: None, + timestamp: Some(TimestampConfig { + period: 5, // 5 second block time. + }), balances: Some(BalancesConfig { transaction_base_fee: 1, transaction_byte_fee: 0, @@ -42,63 +43,6 @@ fn testnet_genesis(initial_authorities: Vec, endowed_accounts: Vec< reclaim_rebate: 0, balances: endowed_accounts.iter().map(|&k|(k, (1 << 60))).collect(), }), - session: Some(SessionConfig { - validators: initial_authorities.iter().cloned().map(Into::into).collect(), - session_length: 10, - }), - staking: Some(StakingConfig { - current_era: 0, - intentions: initial_authorities.iter().cloned().map(Into::into).collect(), - minimum_validator_count: 1, - validator_count: 2, - sessions_per_era: 5, - bonding_duration: 2 * 60 * 12, - offline_slash: Perbill::zero(), - session_reward: Perbill::zero(), - current_offline_slash: 0, - current_session_reward: 0, - offline_slash_grace: 0, - }), - democracy: Some(DemocracyConfig { - launch_period: 9, - voting_period: 18, - minimum_deposit: 10, - }), - council_seats: Some(CouncilSeatsConfig { - active_council: endowed_accounts.iter() - .filter(|a| initial_authorities.iter().find(|&b| a.0 == b.0).is_none()) - .map(|a| (a.clone(), 1000000)).collect(), - candidacy_bond: 10, - voter_bond: 2, - present_slash_per_voter: 1, - carry_count: 4, - presentation_duration: 10, - approval_voting_period: 20, - term_duration: 1000000, - desired_seats: (endowed_accounts.len() - initial_authorities.len()) as u32, - inactive_grace_period: 1, - }), - council_voting: Some(CouncilVotingConfig { - cooloff_period: 75, - voting_period: 20, - }), - timestamp: Some(TimestampConfig { - period: 5, // 5 second block time. - }), - treasury: Some(TreasuryConfig { - proposal_bond: Permill::from_percent(5), - proposal_bond_minimum: 1_000_000, - spend_period: 12 * 60 * 24, - burn: Permill::from_percent(50), - }), - contract: Some(ContractConfig { - contract_fee: 21, - call_base_fee: 135, - create_base_fee: 175, - gas_price: 1, - max_depth: 1024, - block_gas_limit: 10_000_000, - }), } } From c4aa0c900851cf4ea89e0152a5bd5469fc2f7f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 15 Oct 2018 19:23:49 +0200 Subject: [PATCH 2/2] Finish moving to `check_inherents` --- consensus/src/evaluation.rs | 7 ++- consensus/src/lib.rs | 50 ++++++++++++--------- primitives/src/lib.rs | 10 +++++ runtime/Cargo.toml | 4 +- runtime/src/lib.rs | 88 ++++++++++++++++++++++++++++++------- 5 files changed, 117 insertions(+), 42 deletions(-) diff --git a/consensus/src/evaluation.rs b/consensus/src/evaluation.rs index 72a90cdb0..729ce62f0 100644 --- a/consensus/src/evaluation.rs +++ b/consensus/src/evaluation.rs @@ -20,7 +20,7 @@ use super::MAX_TRANSACTIONS_SIZE; use codec::{Decode, Encode}; use node_runtime::{Block as GenericBlock}; -use node_primitives::{Hash, BlockNumber, Timestamp}; +use node_primitives::{Hash, BlockNumber}; use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, As}; @@ -52,7 +52,6 @@ error_chain! { /// upon any initial validity checks failing. pub fn evaluate_initial( proposal: &Block, - now: Timestamp, // TODO: should be InherentData parent_hash: &Hash, parent_number: <::Header as HeaderT>::Number, ) -> Result<()> @@ -80,7 +79,7 @@ where bail!(ErrorKind::WrongNumber(parent_number.as_() + 1, proposal.header.number)); } - // TODO: Should use + // TODO: Should use - Ok(proposal) + Ok(()) } diff --git a/consensus/src/lib.rs b/consensus/src/lib.rs index 4fbb233ae..e986589a4 100644 --- a/consensus/src/lib.rs +++ b/consensus/src/lib.rs @@ -49,7 +49,7 @@ use client::{Client as SubstrateClient, CallExecutor}; use client::runtime_api::{Core, BlockBuilder as BlockBuilderAPI, Miscellaneous, OldTxQueue}; use codec::{Decode, Encode}; use node_primitives::{AccountId, Timestamp, SessionKey, InherentData}; -use node_runtime::Runtime; +use node_runtime::{block_builder_ext::{Error as BBEError, BlockBuilderExt}, Runtime}; use primitives::{AuthorityId, ed25519, Blake2Hasher}; use runtime_primitives::traits::{Block as BlockT, Hash as HashT, Header as HeaderT, As, BlockNumberToHash}; use runtime_primitives::generic::{BlockId, Era}; @@ -88,6 +88,7 @@ pub trait AuthoringApi: Send + Sync + BlockBuilderAPI<::Block, Error=::Error> + + BlockBuilderExt<::Block, Error=::Error> + Core<::Block, AuthorityId, Error=::Error> + Miscellaneous<::Block, Error=::Error> + OldTxQueue<::Block, Error=::Error> @@ -347,7 +348,6 @@ impl bft::Proposer<::Block> for Proposer where assert!(evaluation::evaluate_initial( &substrate_block, - timestamp, &self.parent_hash, self.parent_number, ).is_ok()); @@ -358,34 +358,49 @@ impl bft::Proposer<::Block> for Proposer where fn evaluate(&self, unchecked_proposal: &::Block) -> Self::Evaluate { debug!(target: "bft", "evaluating block on top of parent ({}, {:?})", self.parent_number, self.parent_hash); - let current_timestamp = current_timestamp(); - // do initial serialization and structural integrity checks. - let maybe_proposal = evaluation::evaluate_initial( + match evaluation::evaluate_initial( unchecked_proposal, - current_timestamp, &self.parent_hash, self.parent_number, - ); - - let proposal = match maybe_proposal { + ) { Ok(p) => p, Err(e) => { // TODO: these errors are easily re-checked in runtime. - debug!(target: "bft", "Invalid proposal: {:?}", e); + debug!(target: "bft", "Invalid proposal (initial evaluation failed): {:?}", e); return Box::new(future::ok(false)); } }; - let vote_delays = { - let now = Instant::now(); + let current_timestamp = current_timestamp(); + let inherent = InherentData::new( + current_timestamp, + self.offline.read().reports(&self.validators) + ); + let proposed_timestamp = match self.client.check_inherents( + &self.parent_id, + &unchecked_proposal, + &inherent + ) { + Ok(Ok(())) => None, + Ok(Err(BBEError::TimestampInFuture(timestamp))) => Some(timestamp), + Ok(Err(e)) => { + debug!(target: "bft", "Invalid proposal (check_inherents): {:?}", e); + return Box::new(future::ok(false)); + }, + Err(e) => { + debug!(target: "bft", "Could not call into runtime: {:?}", e); + return Box::new(future::ok(false)); + } + }; + let vote_delays = { // the duration until the given timestamp is current - let proposed_timestamp = ::std::cmp::max(self.minimum_timestamp, proposal.timestamp()); + let proposed_timestamp = ::std::cmp::max(self.minimum_timestamp, proposed_timestamp.unwrap_or(0)); let timestamp_delay = if proposed_timestamp > current_timestamp { let delay_s = proposed_timestamp - current_timestamp; debug!(target: "bft", "Delaying evaluation of proposal for {} seconds", delay_s); - Some(now + Duration::from_secs(delay_s)) + Some(Instant::now() + Duration::from_secs(delay_s)) } else { None }; @@ -398,13 +413,6 @@ impl bft::Proposer<::Block> for Proposer where } }; - // refuse to vote if this block says a validator is offline that we - // think isn't. - let offline = proposal.noted_offline(); - if !self.offline.read().check_consistency(&self.validators[..], offline) { - return Box::new(futures::empty()); - } - // evaluate whether the block is actually valid. // TODO: is it better to delay this until the delays are finished? let evaluated = match self.client.execute_block(&self.parent_id, &unchecked_proposal.clone()).map_err(Error::from) { diff --git a/primitives/src/lib.rs b/primitives/src/lib.rs index 4bd4d4774..c6802c716 100644 --- a/primitives/src/lib.rs +++ b/primitives/src/lib.rs @@ -88,3 +88,13 @@ pub struct InherentData { /// Indices of offline validators. pub offline_indices: Vec, } + +impl InherentData { + /// Create a new `InherentData` instance. + pub fn new(timestamp: Timestamp, offline_indices: Vec) -> Self { + Self { + timestamp, + offline_indices + } + } +} diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index de84c7034..4ee1fb1f9 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -29,6 +29,7 @@ srml-staking = { git = "https://github.com/paritytech/substrate" } srml-system = { git = "https://github.com/paritytech/substrate" } srml-timestamp = { git = "https://github.com/paritytech/substrate" } srml-treasury = { git = "https://github.com/paritytech/substrate" } +substrate-client = { git = "https://github.com/paritytech/substrate", optional = true } sr-version = { git = "https://github.com/paritytech/substrate" } node-primitives = { path = "../primitives" } @@ -57,5 +58,6 @@ std = [ "node-primitives/std", "serde_derive", "serde/std", - "safe-mix/std" + "safe-mix/std", + "substrate-client" ] diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 60659ea8b..1fa70bbd7 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -58,7 +58,7 @@ extern crate sr_version as version; extern crate node_primitives; use rstd::prelude::*; -use node_primitives::{AccountId, AccountIndex, Balance, BlockNumber, Hash, Index, SessionKey, Signature, InherentData}; +use node_primitives::{AccountId, AccountIndex, Balance, BlockNumber, Hash, Index, SessionKey, Signature, InherentData, Timestamp as TimestampType}; use runtime_api::runtime::*; use runtime_primitives::ApplyResult; use runtime_primitives::transaction_validity::TransactionValidity; @@ -77,7 +77,6 @@ pub use runtime_primitives::{Permill, Perbill}; pub use timestamp::BlockPeriod; pub use srml_support::StorageValue; #[cfg(any(feature = "std", test))] -pub use checked_block::CheckedBlock; const TIMESTAMP_SET_POSITION: u32 = 0; const NOTE_OFFLINE_POSITION: u32 = 1; @@ -187,6 +186,55 @@ pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. pub type Executive = executive::Executive; + +//TODO: Move upstream into `BlockBuilder`. +pub mod block_builder_ext { + #[cfg(feature = "std")] + extern crate substrate_client as client; + #[cfg(feature = "std")] + extern crate parity_codec; + + use super::BlockT; + use rstd::borrow::Cow; + #[cfg(feature = "std")] + use substrate_primitives::Blake2Hasher; + #[cfg(feature = "std")] + use runtime_primitives::generic::BlockId; + + #[derive(Encode, Decode, Debug)] + pub enum Error { + Generic(Cow<'static, str>), + TimestampInFuture(u64), + } + + decl_apis! { + pub trait BlockBuilderExt { + fn check_inherents(block: Block, data: InherentData) -> Result<(), Error>; + } + } + + #[cfg(feature = "std")] + impl BlockBuilderExt for client::Client where + B: client::backend::Backend, + E: client::CallExecutor, + Block: BlockT, + { + type Error = client::error::Error; + + fn check_inherents( + &self, + at: &BlockId, + block: &Block, + data: &InherentData + ) -> Result, Self::Error> { + self.call_api_at(at, "check_inherents", &(block, data)) + } + } + +} +use block_builder_ext::runtime::BlockBuilderExt; +use block_builder_ext::Error as BBEError; + impl_apis! { impl Core for Runtime { fn version() -> RuntimeVersion { @@ -236,34 +284,42 @@ impl_apis! { inherent } - fn check_inherents(block: Block, data: InherentData) -> Result<(), &'static str> { + fn random_seed() -> ::Hash { + System::random_seed() + } + } + + impl BlockBuilderExt for Runtime { + fn check_inherents(block: Block, data: InherentData) -> Result<(), BBEError> { // TODO: v1: should be automatically gathered // Timestamp module... - const MAX_TIMESTAMP_DRIFT: Timestamp = 60; + const MAX_TIMESTAMP_DRIFT: TimestampType = 60; let xt = block.extrinsics.get(TIMESTAMP_SET_POSITION as usize) - .ok_or("No valid timestamp inherent in block")?; - let t = match (xt.is_signed(), xt.function) { + .ok_or_else(|| BBEError::Generic("No valid timestamp inherent in block".into()))?; + let t = match (xt.is_signed(), &xt.function) { (false, Call::Timestamp(TimestampCall::set(t))) => t, - _ => bail!("No valid timestamp inherent in block"), + _ => return Err(BBEError::Generic("No valid timestamp inherent in block".into())), }; - if t > now + MAX_TIMESTAMP_DRIFT { - bail!("Timestamp in future") + + if *t > data.timestamp + MAX_TIMESTAMP_DRIFT { + return Err(BBEError::TimestampInFuture(*t)) } // Offline indices let noted_offline = - self.inner.extrinsics.get(NOTE_OFFLINE_POSITION as usize).and_then(|xt| match xt.function { + block.extrinsics.get(NOTE_OFFLINE_POSITION as usize).and_then(|xt| match xt.function { Call::Consensus(ConsensusCall::note_offline(ref x)) => Some(&x[..]), _ => None, }).unwrap_or(&[]); - for i in noted_offline { - if data.offline_indices.iter().find(i).is_none() { - bail!("Online node marked offline") - } - } - Ok(()) + noted_offline.iter().try_for_each(|n| + if !data.offline_indices.contains(n) { + Err(BBEError::Generic("Online node marked offline".into())) + } else { + Ok(()) + } + ) } }