CheckInherentError can now report when something would be valid (#1204)

* CheckInherentError can now report when something would be valid

* set timestamp inherent to next valid block time

* return max timestamp for valid-after when checking

substrate/Cargo.lock

@@ -2972,7 +2972,6 @@ version = "0.1.0"
 dependencies = [
  "hex-literal 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-codec 2.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
- "parity-codec-derive 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.80 (registry+https://github.com/rust-lang/crates.io-index)",
  "sr-io 0.1.0",
  "sr-primitives 0.1.0",

substrate/core/consensus/rhd/src/lib.rs

@@ -1177,7 +1177,7 @@ impl<C, A> BaseProposer<<C as AuthoringApi>::Block> for Proposer<C, A> where
 			&inherent,
 		) {
 			Ok(Ok(())) => None,
-			Ok(Err(BlockBuilderError::TimestampInFuture(timestamp))) => Some(timestamp),
+			Ok(Err(BlockBuilderError::ValidAtTimestamp(timestamp))) => Some(timestamp),
 			Ok(Err(e)) => {
 				debug!(target: "rhd", "Invalid proposal (check_inherents): {:?}", e);
 				return Box::new(future::ok(false));

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

@@ -487,7 +487,10 @@ impl BasicInherentData {
 #[derive(Encode)]
 #[cfg_attr(feature = "std", derive(Decode))]
 pub enum CheckInherentError {
-	TimestampInFuture(u64),
+	/// The inherents are generally valid but a delay until the given timestamp
+	/// is required.
+	ValidAtTimestamp(u64),
+	/// Some other error has occurred.
 	Other(RuntimeString),
 }
 

substrate/srml/support/src/inherent.rs

@@ -15,7 +15,7 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 #[doc(hidden)]
-pub use rstd::{result::Result, vec::Vec};
+pub use rstd::{cmp, result::Result, vec::Vec};
 #[doc(hidden)]
 pub use runtime_primitives::{
 	traits::{ProvideInherent, Block as BlockT}, CheckInherentError
@@ -52,17 +52,32 @@ macro_rules! impl_outer_inherent {
 				block: $block,
 				data: $inherent
 			) -> $crate::inherent::Result<(), $crate::inherent::CheckInherentError> {
+				use $crate::inherent::CheckInherentError;
+
+				let mut max_valid_after = None;
 				$(
-					<$module_ty as $crate::inherent::ProvideInherent>::check_inherent(
+					let res = <$module_ty as $crate::inherent::ProvideInherent>::check_inherent(
 						&block,
 						data.$module,
 						&|xt| match xt.function {
 							Call::$module_ty(ref data) => Some(data),
 							_ => None,
 						},
-					)?;
+					);
+
+					match res {
+						Err(CheckInherentError::ValidAtTimestamp(t)) =>
+							max_valid_after = $crate::inherent::cmp::max(max_valid_after, Some(t)),
+						res => res?
+					}
 				)*
-				Ok(())
+
+				// once everything else has checked out, take the maximum of
+				// all things which are timestamp-restricted.
+				match max_valid_after {
+					Some(t) => Err(CheckInherentError::ValidAtTimestamp(t)),
+					None => Ok(())
+				}
 			}
 		}
 	};

substrate/srml/timestamp/Cargo.toml

@@ -6,7 +6,6 @@ authors = ["Parity Technologies <admin@parity.io>"]
 [dependencies]
 hex-literal = "0.1.0"
 serde = { version = "1.0", default-features = false }
-parity-codec-derive = { version = "2.1", default-features = false }
 parity-codec = { version = "2.1", default-features = false }
 substrate-primitives = { path = "../../core/primitives", default-features = false }
 sr-std = { path = "../../core/sr-std", default-features = false }
@@ -28,7 +27,6 @@ std = [
 	"sr-primitives/std",
 	"srml-consensus/std",
 	"serde/std",
-	"parity-codec-derive/std",
 	"parity-codec/std",
 	"substrate-primitives/std",
 	"srml-system/std",

substrate/srml/timestamp/src/lib.rs

@@ -46,8 +46,6 @@ extern crate sr_primitives as runtime_primitives;
 extern crate srml_system as system;
 extern crate srml_consensus as consensus;
 extern crate parity_codec as codec;
-#[macro_use]
-extern crate parity_codec_derive;
 
 use codec::HasCompact;
 use runtime_support::{StorageValue, Parameter};
@@ -136,7 +134,8 @@ impl<T: Trait> ProvideInherent for Module<T> {
 	type Call = Call<T>;
 
 	fn create_inherent_extrinsics(data: Self::Inherent) -> Vec<(u32, Self::Call)> {
-		vec![(T::TIMESTAMP_SET_POSITION, Call::set(data.into()))]
+		let next_time = ::rstd::cmp::max(data, Self::now() + Self::block_period());
+		vec![(T::TIMESTAMP_SET_POSITION, Call::set(next_time.into()))]
 	}
 
 	fn check_inherent<Block: BlockT, F: Fn(&Block::Extrinsic) -> Option<&Self::Call>>(
@@ -152,8 +151,11 @@ impl<T: Trait> ProvideInherent for Module<T> {
 			_ => return Err(CheckInherentError::Other("No valid timestamp inherent in block".into())),
 		}.into().as_();
 
+		let minimum = (Self::now() + Self::block_period()).as_();
 		if t > data.as_() + MAX_TIMESTAMP_DRIFT {
-			Err(CheckInherentError::TimestampInFuture(t))
+			Err(CheckInherentError::Other("Timestamp too far in future to accept".into()))
+		} else if t < minimum {
+			Err(CheckInherentError::ValidAtTimestamp(minimum))
 		} else {
 			Ok(())
 		}