From 9bae56edc12377a160d1e56b8a2c95831d666abc Mon Sep 17 00:00:00 2001
From: Peter Goodspeed-Niklaus <peter.r.goodspeedniklaus@gmail.com>
Date: Fri, 3 Jul 2020 17:03:59 +0200
Subject: [PATCH] extract provisioner data fetch

This satisfies two requirements:

- only applies the timeout to actually fetching the provisioner data,
  not to constructing the block after
- simplifies the problem of injecting default data if we could not
  get the real provisioner data in time.
---
 node/core/proposer/src/lib.rs | 91 ++++++++++++++++++++++++-----------
 primitives/src/parachain.rs   |  2 +-
 2 files changed, 64 insertions(+), 29 deletions(-)

diff --git a/node/core/proposer/src/lib.rs b/node/core/proposer/src/lib.rs
index 47a31dd35d57..b53ac5729fa9 100644
--- a/node/core/proposer/src/lib.rs
+++ b/node/core/proposer/src/lib.rs
@@ -1,6 +1,6 @@
 use futures::prelude::*;
 use futures::select;
-use polkadot_node_subsystem::{messages::{AllMessages, ProvisionerMessage}, SubsystemError};
+use polkadot_node_subsystem::{messages::{AllMessages, ProvisionerInherentData, ProvisionerMessage}, SubsystemError};
 use polkadot_overseer::OverseerHandler;
 use polkadot_primitives::{
 	inclusion_inherent,
@@ -93,7 +93,8 @@ pub struct Proposer<TxPool: TransactionPool<Block = Block>, Backend, Client> {
 	parent_header_hash: Hash,
 }
 
-impl<TxPool, Backend, Client> sp_consensus::Proposer<Block> for Proposer<TxPool, Backend, Client>
+// This impl has the same generic bounds as the Proposer impl.
+impl<TxPool, Backend, Client> Proposer<TxPool, Backend, Client>
 where
 	TxPool: 'static + TransactionPool<Block = Block>,
 	Client: 'static
@@ -109,24 +110,15 @@ where
 	// Rust bug: https://github.com/rust-lang/rust/issues/24159
 	sp_api::StateBackendFor<Client, Block>: sp_api::StateBackend<HashFor<Block>> + Send,
 {
-	type Transaction = sc_client_api::TransactionFor<Backend, Block>;
-	type Proposal = Pin<Box<
-		dyn Future<Output = Result<Proposal<Block, sp_api::TransactionFor<Client, Block>>, Error>> + Send,
-	>>;
-	type Error = Error;
-
-	fn propose(
-		self,
-		mut inherent_data: InherentData,
-		inherent_digests: DigestFor<Block>,
-		max_duration: time::Duration,
-		record_proof: RecordProof,
-	) -> Self::Proposal {
+	/// Get provisioner inherent data
+	///
+	/// This function has a constant timeout: `PROPOSE_TIMEOUT`.
+	fn get_provisioner_data(&self) -> impl Future<Output = Result<ProvisionerInherentData, Error>> {
 		// clone this (lightweight) data because we're going to move it into the future
 		let mut overseer = self.overseer.clone();
 		let parent_header_hash = self.parent_header_hash.clone();
 
-		let mut proposal = async move {
+		let mut provisioner_inherent_data = async move {
 			let (sender, receiver) = futures::channel::oneshot::channel();
 
 			// strictly speaking, we don't _have_ to .await this send_msg before opening the
@@ -137,17 +129,7 @@ where
 				ProvisionerMessage::RequestInherentData(parent_header_hash, sender),
 			)).await?;
 
-			let provisioner_inherent_data = receiver.await.map_err(Error::ClosedChannelFromProvisioner)?;
-
-			inherent_data.put_data(
-				inclusion_inherent::INHERENT_IDENTIFIER,
-				&provisioner_inherent_data,
-			)?;
-
-			self.inner
-				.propose(inherent_data, inherent_digests, max_duration, record_proof)
-				.await
-				.map_err(Into::into)
+			receiver.await.map_err(Error::ClosedChannelFromProvisioner)
 		}
 		.boxed()
 		.fuse();
@@ -156,10 +138,63 @@ where
 
 		async move {
 			select! {
-				proposal_result = proposal => proposal_result,
+				pid = provisioner_inherent_data => pid,
 				_ = timeout => Err(Error::Timeout),
 			}
 		}
+	}
+}
+
+impl<TxPool, Backend, Client> sp_consensus::Proposer<Block> for Proposer<TxPool, Backend, Client>
+where
+	TxPool: 'static + TransactionPool<Block = Block>,
+	Client: 'static
+		+ BlockBuilderProvider<Backend, Block, Client>
+		+ ProvideRuntimeApi<Block>
+		+ HeaderBackend<Block>
+		+ Send
+		+ Sync,
+	Client::Api:
+		ParachainHost<Block> + BlockBuilderApi<Block> + ApiExt<Block, Error = sp_blockchain::Error>,
+	Backend:
+		'static + sc_client_api::Backend<Block, State = sp_api::StateBackendFor<Client, Block>>,
+	// Rust bug: https://github.com/rust-lang/rust/issues/24159
+	sp_api::StateBackendFor<Client, Block>: sp_api::StateBackend<HashFor<Block>> + Send,
+{
+	type Transaction = sc_client_api::TransactionFor<Backend, Block>;
+	type Proposal = Pin<Box<
+		dyn Future<Output = Result<Proposal<Block, sp_api::TransactionFor<Client, Block>>, Error>> + Send,
+	>>;
+	type Error = Error;
+
+	fn propose(
+		self,
+		mut inherent_data: InherentData,
+		inherent_digests: DigestFor<Block>,
+		max_duration: time::Duration,
+		record_proof: RecordProof,
+	) -> Self::Proposal {
+		let provisioner_data = self.get_provisioner_data();
+
+		async move {
+			let provisioner_data = match provisioner_data.await {
+				Ok(pd) => pd,
+				Err(err) => {
+					log::warn!("could not get provisioner inherent data; injecting default data: {}", err);
+					Default::default()
+				}
+			};
+
+			inherent_data.put_data(
+				inclusion_inherent::INHERENT_IDENTIFIER,
+				&provisioner_data,
+			)?;
+
+			self.inner
+				.propose(inherent_data, inherent_digests, max_duration, record_proof)
+				.await
+				.map_err(Into::into)
+		}
 		.boxed()
 	}
 }
diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs
index 33a0f0363d46..03448a29fa70 100644
--- a/primitives/src/parachain.rs
+++ b/primitives/src/parachain.rs
@@ -724,7 +724,7 @@ impl From<BitVec<bitvec::order::Lsb0, u8>> for AvailabilityBitfield {
 pub type SignedAvailabilityBitfield = Signed<AvailabilityBitfield>;
 
 /// A set of signed availability bitfields. Should be sorted by validator index, ascending.
-#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)]
+#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, Default)]
 pub struct SignedAvailabilityBitfields(pub Vec<SignedAvailabilityBitfield>);
 
 impl From<Vec<SignedAvailabilityBitfield>> for SignedAvailabilityBitfields {