Merge pull request #1 from paritytech/gav-remove-checked-block

Groundwork to remove `checked_block`

consensus/src/evaluation.rs

@@ -19,8 +19,8 @@
 use super::MAX_TRANSACTIONS_SIZE;
 
 use codec::{Decode, Encode};
-use node_runtime::{Block as GenericBlock, CheckedBlock};
-use node_primitives::{Hash, BlockNumber, Timestamp};
+use node_runtime::{Block as GenericBlock};
+use node_primitives::{Hash, BlockNumber};
 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,15 @@ error_chain! {
 /// upon any initial validity checks failing.
 pub fn evaluate_initial<Block: BlockT, Hash>(
 	proposal: &Block,
-	now: Timestamp,
 	parent_hash: &Hash,
 	parent_number: <<Block as BlockT>::Header as HeaderT>::Number,
-) -> Result<CheckedBlock>
+) -> Result<()>
 where
 	Hash: PartialEq<<<GenericBlock as BlockT>::Header as HeaderT>::Hash>,
 	Hash: Into<self::Hash> + 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 +79,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)
+	Ok(())
 }

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<<Self as AuthoringApi>::Block, Error=<Self as AuthoringApi>::Error>
+	+ BlockBuilderExt<<Self as AuthoringApi>::Block, Error=<Self as AuthoringApi>::Error>
 	+ Core<<Self as AuthoringApi>::Block, AuthorityId, Error=<Self as AuthoringApi>::Error>
 	+ Miscellaneous<<Self as AuthoringApi>::Block, Error=<Self as AuthoringApi>::Error>
 	+ OldTxQueue<<Self as AuthoringApi>::Block, Error=<Self as AuthoringApi>::Error>
@@ -347,7 +348,6 @@ impl<C, A> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C, A> where
 
 		assert!(evaluation::evaluate_initial(
 			&substrate_block,
-			timestamp,
 			&self.parent_hash,
 			self.parent_number,
 		).is_ok());
@@ -358,34 +358,49 @@ impl<C, A> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C, A> where
 	fn evaluate(&self, unchecked_proposal: &<C as AuthoringApi>::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<C, A> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C, A> 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) {
@@ -481,9 +489,9 @@ impl<C, A> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C, A> 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: <<C as AuthoringApi>::Block as BlockT>::Extrinsic = Decode::decode(&mut extrinsic.encode().as_slice()).expect("Encoded extrinsic is valid");

primitives/src/lib.rs

@@ -88,3 +88,13 @@ pub struct InherentData {
 	/// Indices of offline validators.
 	pub offline_indices: Vec<u32>,
 }
+
+impl InherentData {
+	/// Create a new `InherentData` instance.
+	pub fn new(timestamp: Timestamp, offline_indices: Vec<u32>) -> Self {
+		Self {
+			timestamp,
+			offline_indices
+		}
+	}
+}

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"
 ]