From d4c19c065262bcc8c64a20b8c30efb7525e34508 Mon Sep 17 00:00:00 2001
From: Marcio Diaz <marcio@parity.io>
Date: Mon, 9 Dec 2019 20:10:41 +0100
Subject: [PATCH] Limit number of uncles that can be provided. (#4298)

* Limit number of uncles that can be provided.

* Check length of uncles vector on inherent.

* Set fatal error to true for too many uncles.

* Take max uncles in create_inherent.
---
 substrate/Cargo.lock                       |  1 +
 substrate/frame/authorship/src/lib.rs      | 26 ++++++++++++++++------
 substrate/primitives/authorship/Cargo.toml |  1 +
 substrate/primitives/authorship/src/lib.rs | 18 ++++++++++++++-
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock
index e85290ceec..69030e5357 100644
--- a/substrate/Cargo.lock
+++ b/substrate/Cargo.lock
@@ -6035,6 +6035,7 @@ version = "2.0.0"
 dependencies = [
  "parity-scale-codec 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "sp-inherents 2.0.0",
+ "sp-runtime 2.0.0",
  "sp-std 2.0.0",
 ]
 
diff --git a/substrate/frame/authorship/src/lib.rs b/substrate/frame/authorship/src/lib.rs
index 1f918e093a..73a38b0ead 100644
--- a/substrate/frame/authorship/src/lib.rs
+++ b/substrate/frame/authorship/src/lib.rs
@@ -22,7 +22,7 @@
 
 use rstd::{result, prelude::*};
 use rstd::collections::btree_set::BTreeSet;
-use support::{decl_module, decl_storage};
+use support::{decl_module, decl_storage, ensure};
 use support::traits::{FindAuthor, VerifySeal, Get};
 use support::dispatch::Result as DispatchResult;
 use codec::{Encode, Decode};
@@ -30,9 +30,9 @@ use system::ensure_none;
 use sp_runtime::traits::{Header as HeaderT, One, Zero};
 use support::weights::SimpleDispatchInfo;
 use inherents::{InherentIdentifier, ProvideInherent, InherentData, MakeFatalError};
-use sp_authorship::{
-	INHERENT_IDENTIFIER, UnclesInherentData,
-};
+use sp_authorship::{INHERENT_IDENTIFIER, UnclesInherentData, InherentError};
+
+const MAX_UNCLES: usize = 10;
 
 pub trait Trait: system::Trait {
 	/// Find the author of a block.
@@ -187,6 +187,7 @@ decl_module! {
 		#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
 		fn set_uncles(origin, new_uncles: Vec<T::Header>) -> DispatchResult {
 			ensure_none(origin)?;
+			ensure!(new_uncles.len() <= MAX_UNCLES, "Too many uncles");
 
 			if <Self as Store>::DidSetUncles::get() {
 				return Err("Uncles already set in block.");
@@ -314,7 +315,7 @@ impl<T: Trait> Module<T> {
 
 impl<T: Trait> ProvideInherent for Module<T> {
 	type Call = Call<T>;
-	type Error = MakeFatalError<()>;
+	type Error = InherentError;
 	const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER;
 
 	fn create_inherent(data: &InherentData) -> Option<Self::Call> {
@@ -338,6 +339,10 @@ impl<T: Trait> ProvideInherent for Module<T> {
 						let hash = uncle.hash();
 						set_uncles.push(uncle);
 						existing_hashes.push(hash);
+
+						if set_uncles.len() == MAX_UNCLES {
+							break
+						}
 					}
 					Err(_) => {
 						// skip this uncle
@@ -353,8 +358,15 @@ impl<T: Trait> ProvideInherent for Module<T> {
 		}
 	}
 
-	fn check_inherent(_call: &Self::Call, _data: &InherentData) -> result::Result<(), Self::Error> {
-		Ok(())
+	fn check_inherent(call: &Self::Call, _data: &InherentData) -> result::Result<(), Self::Error> {
+		match call {
+			Call::set_uncles(ref uncles) if uncles.len() > MAX_UNCLES => {
+				Err(InherentError::Uncles("Too many uncles".into()))
+			},
+			_ => {
+				Ok(())
+			},
+		}
 	}
 }
 
diff --git a/substrate/primitives/authorship/Cargo.toml b/substrate/primitives/authorship/Cargo.toml
index 3b9604c688..4db66e6421 100644
--- a/substrate/primitives/authorship/Cargo.toml
+++ b/substrate/primitives/authorship/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2018"
 
 [dependencies]
 sp-inherents = { package = "sp-inherents", path = "../inherents", default-features = false }
+sr-primitives = { package = "sp-runtime", path = "../sr-primitives", default-features = false }
 rstd = { package = "sp-std", path = "../sr-std", default-features = false }
 codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] }
 
diff --git a/substrate/primitives/authorship/src/lib.rs b/substrate/primitives/authorship/src/lib.rs
index e381c97094..b80102f41c 100644
--- a/substrate/primitives/authorship/src/lib.rs
+++ b/substrate/primitives/authorship/src/lib.rs
@@ -21,11 +21,27 @@
 use rstd::{result::Result, prelude::*};
 
 use codec::{Encode, Decode};
-use sp_inherents::{Error, InherentIdentifier, InherentData};
+use sp_inherents::{Error, InherentIdentifier, InherentData, IsFatalError};
+use sr_primitives::RuntimeString;
 
 /// The identifier for the `uncles` inherent.
 pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"uncles00";
 
+/// Errors that can occur while checking the authorship inherent.
+#[derive(Encode, sr_primitives::RuntimeDebug)]
+#[cfg_attr(feature = "std", derive(Decode))]
+pub enum InherentError {
+	Uncles(RuntimeString),
+}
+
+impl IsFatalError for InherentError {
+	fn is_fatal_error(&self) -> bool {
+		match self {
+			InherentError::Uncles(_) => true,
+		}
+	}
+}
+
 /// Auxiliary trait to extract uncles inherent data.
 pub trait UnclesInherentData<H: Decode> {
 	/// Get uncles.