Allow modules to indicate full block. (#1687)

* Add transaction skipping.

* Update runtime.

substrate/core/basic-authorship/src/basic_authorship.rs

@@ -20,7 +20,7 @@
 //
 use std::{self, time, sync::Arc};
 
-use log::{info, debug, trace};
+use log::{info, debug};
 
 use client::{
 	self, error, Client as SubstrateClient, CallExecutor,
@@ -190,10 +190,17 @@ impl<Block, C, A> Proposer<Block, C, A>	where
 	{
 		use runtime_primitives::traits::BlakeTwo256;
 
+		/// If the block is full we will attempt to push at most
+		/// this number of transactions before quitting for real.
+		/// It allows us to increase block utilisation.
+		const MAX_SKIPPED_TRANSACTIONS: usize = 8;
+
 		let block = self.client.build_block(
 			&self.parent_id,
 			inherent_data,
 			|block_builder| {
+				let mut is_first = true;
+				let mut skipped = 0;
 				let mut unqueue_invalid = Vec::new();
 				let pending_iterator = self.transaction_pool.ready();
 
@@ -208,14 +215,27 @@ impl<Block, C, A> Proposer<Block, C, A>	where
 							debug!("[{:?}] Pushed to the block.", pending.hash);
 						}
 						Err(error::Error(error::ErrorKind::ApplyExtrinsicFailed(ApplyError::FullBlock), _)) => {
-							debug!("Block is full, proceed with proposing.");
+							if is_first {
-							break;
+								debug!("[{:?}] Invalid transaction: FullBlock on empty block", pending.hash);
+								unqueue_invalid.push(pending.hash.clone());
+							} else if skipped < MAX_SKIPPED_TRANSACTIONS {
+								skipped += 1;
+								debug!(
+									"Block seems full, but will try {} more transactions before quitting.",
+									MAX_SKIPPED_TRANSACTIONS - skipped
+								);
+							} else {
+								debug!("Block is full, proceed with proposing.");
+								break;
+							}
 						}
 						Err(e) => {
-							trace!(target: "transaction-pool", "Invalid transaction: {}", e);
+							debug!("[{:?}] Invalid transaction: {}", pending.hash, e);
 							unqueue_invalid.push(pending.hash.clone());
 						}
 					}
+
+					is_first = false;
 				}
 
 				self.transaction_pool.remove_invalid(&unqueue_invalid);

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

@@ -14,8 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-//! System manager: Handles all of the top-level stuff; executing block/transaction, setting code
+//! Runtime Modules shared primitive types.
-//! and depositing logs.
 
 #![warn(missing_docs)]
 
@@ -44,6 +43,16 @@ pub mod traits;
 pub mod generic;
 pub mod transaction_validity;
 
+/// Full block error message.
+///
+/// This allows modules to indicate that given transaction is potentially valid
+/// in the future, but can't be executed in the current state.
+/// Note this error should be returned early in the execution to prevent DoS,
+/// cause the fees are not being paid if this error is returned.
+///
+/// Example: block gas limit is reached (the transaction can be retried in the next block though).
+pub const BLOCK_FULL: &str = "block size limit is reached";
+
 /// Justification type.
 pub type Justification = Vec<u8>;
 

substrate/srml/contract/src/gas.rs

@@ -16,6 +16,7 @@
 
 use crate::{GasSpent, Module, Trait};
 use balances;
+use runtime_primitives::BLOCK_FULL;
 use runtime_primitives::traits::{As, CheckedMul, CheckedSub, Zero};
 use runtime_support::StorageValue;
 
@@ -206,7 +207,8 @@ pub fn buy_gas<T: Trait>(
 	// This cannot underflow since `gas_spent` is never greater than `block_gas_limit`.
 	let gas_available = <Module<T>>::block_gas_limit() - <Module<T>>::gas_spent();
 	if gas_limit > gas_available {
-		return Err("block gas limit is reached");
+		// gas limit reached, revert the transaction and retry again in the future
+		return Err(BLOCK_FULL);
 	}
 
 	// Buy the specified amount of gas.

substrate/srml/executive/src/lib.rs

@@ -207,7 +207,10 @@ impl<
 		let r = f.dispatch(s.into());
 		<system::Module<System>>::note_applied_extrinsic(&r, encoded_len as u32);
 
-		r.map(|_| internal::ApplyOutcome::Success).or_else(|e| Ok(internal::ApplyOutcome::Fail(e)))
+		r.map(|_| internal::ApplyOutcome::Success).or_else(|e| match e {
+			primitives::BLOCK_FULL => Err(internal::ApplyError::FullBlock),
+			e => Ok(internal::ApplyOutcome::Fail(e))
+		})
 	}
 
 	fn final_checks(header: &System::Header) {

substrate/srml/support/src/lib.rs

@@ -56,8 +56,8 @@ mod double_map;
 pub use self::storage::{StorageVec, StorageList, StorageValue, StorageMap};
 pub use self::hashable::Hashable;
 pub use self::dispatch::{Parameter, Dispatchable, Callable, IsSubType};
+pub use self::double_map::StorageDoubleMap;
 pub use runtime_io::print;
-pub use double_map::StorageDoubleMap;
 
 #[doc(inline)]
 pub use srml_support_procedural::decl_storage;