Fix build & session logic, introduce tests

demo/cli/src/lib.rs

@@ -107,10 +107,10 @@ pub fn run<I, T>(args: I) -> error::Result<()> where
 			authorities: vec![god_key.clone()],
 		}),
 		system: None,
-	//		block_time: 5,			// 5 second block time.
 		session: Some(SessionConfig {
 			validators: vec![god_key.clone().into()],
 			session_length: 720,	// that's 1 hour per session.
+			broken_percent_late: 30,
 		}),
 		staking: Some(StakingConfig {
 			current_era: 0,
@@ -148,8 +148,7 @@ pub fn run<I, T>(args: I) -> error::Result<()> where
 			voting_period: 7 * 120 * 24, // 7 day voting period for council members.
 		}),
 		timestamp: Some(TimestampConfig {
-			now: 0,
-			period: 5,
+			period: 5,					// 5 second block time.
 		}),
 	}.build_storage();
 

demo/runtime/src/lib.rs

@@ -103,6 +103,7 @@ impl Convert<AccountId, SessionKey> for SessionKeyConversion {
 
 impl session::Trait for Concrete {
 	type ConvertAccountIdToSessionKey = SessionKeyConversion;
+	type OnSessionChange = Staking;
 }
 
 /// Session module for this concrete runtime.

polkadot/api/src/full.rs

@@ -112,7 +112,7 @@ impl<B: LocalBackend<Block>> PolkadotApi for Client<B, LocalCallExecutor<B, Nati
 	}
 
 	fn timestamp(&self, at: &CheckedId) -> Result<Timestamp> {
-		with_runtime!(self, at, ::runtime::Timestamp::now)
+		with_runtime!(self, at, ::runtime::Timestamp::get)
 	}
 
 	fn evaluate_block(&self, at: &CheckedId, block: Block) -> Result<bool> {

polkadot/cli/src/preset_config.rs

@@ -22,7 +22,7 @@ use serde_json;
 use substrate_primitives::{AuthorityId, storage::{StorageKey, StorageData}};
 use runtime_primitives::{MakeStorage, BuildStorage, StorageMap};
 use polkadot_runtime::{GenesisConfig, ConsensusConfig, CouncilConfig, DemocracyConfig,
-	SessionConfig, StakingConfig};
+	SessionConfig, StakingConfig, TimestampConfig};
 use chain_spec::ChainSpec;
 
 enum Config {
@@ -99,6 +99,7 @@ impl PresetConfig {
 			session: Some(SessionConfig {
 				validators: initial_authorities.iter().cloned().map(Into::into).collect(),
 				session_length: 720,	// that's 1 hour per session.
+				broken_percent_late: 30,
 			}),
 			staking: Some(StakingConfig {
 				current_era: 0,
@@ -136,7 +137,9 @@ impl PresetConfig {
 				voting_period: 7 * 120 * 24, // 7 day voting period for council members.
 			}),
 			parachains: Some(Default::default()),
-			timestamp: Some(Default::default()),
+			timestamp: Some(TimestampConfig {
+				period: 5,					// 5 second block time.
+			}),
 		});
 		let boot_nodes = vec![
 			"enode://a93a29fa68d965452bf0ff8c1910f5992fe2273a72a1ee8d3a3482f68512a61974211ba32bb33f051ceb1530b8ba3527fc36224ba6b9910329025e6d9153cf50@104.211.54.233:30333".into(),
@@ -164,6 +167,7 @@ impl PresetConfig {
 			session: Some(SessionConfig {
 				validators: initial_authorities.iter().cloned().map(Into::into).collect(),
 				session_length: 10,
+				broken_percent_late: 30,
 			}),
 			staking: Some(StakingConfig {
 				current_era: 0,
@@ -201,7 +205,9 @@ impl PresetConfig {
 				voting_period: 20,
 			}),
 			parachains: Some(Default::default()),
-			timestamp: Some(Default::default()),
+			timestamp: Some(TimestampConfig {
+				period: 5,					// 5 second block time.
+			}),
 		});
 		let boot_nodes = Vec::new();
 		PresetConfig { genesis_config, boot_nodes }

polkadot/runtime/src/lib.rs

@@ -142,6 +142,7 @@ impl Convert<AccountId, SessionKey> for SessionKeyConversion {
 
 impl session::Trait for Concrete {
 	type ConvertAccountIdToSessionKey = SessionKeyConversion;
+	type OnSessionChange = Staking;
 }
 /// Session module for this concrete runtime.
 pub type Session = session::Module<Concrete>;

substrate/runtime/session/src/lib.rs

@@ -90,6 +90,8 @@ decl_storage! {
 	pub CurrentIndex get(current_index): b"ses:ind" => required T::BlockNumber;
 	// Timestamp when current session started.
 	pub CurrentStart get(current_start): b"ses:current_start" => required T::Value;
+	// Percent by which the session must necessarily finish late before we early-exit the session.
+	pub BrokenPercentLate get(broken_percent_late): b"ses:broken_percent_late" => required T::Value;
 
 	// Block at which the session length last changed.
 	LastLengthChange: b"ses:llc" => T::BlockNumber;
@@ -188,17 +190,34 @@ impl<T: Trait> Module<T> {
 		session_length * block_period
 	}
 
+	/// Number of blocks remaining in this session, not counting this one. If the session is
+	/// due to rotate at the end of this block, then it will return 0. If the just began, then
+	/// it will return `Self::length() - 1`.
+	pub fn blocks_remaining() -> T::BlockNumber {
+		// 2 0 2 0 2   0 0
+		// 3 2 3 2 0   1 1
+		// 4 - - 1 1   2 2
+		// 5 - - 0 2   3 0
+		// 6 5 - 2 0   4 1
+		// 7 - - 1 1   5 2
+		let length = Self::length();
+		let length_minus_1 = length - One::one();
+		let block_number = <system::Module<T>>::block_number();
+		length_minus_1 - (block_number - Self::last_length_change() + length_minus_1) % length
+	}
+
 	/// Returns `true` if the current validator set is taking took long to validate blocks.
 	pub fn broken_validation() -> bool {
 		let now = <timestamp::Module<T>>::get();
 		let block_period = <timestamp::Module<T>>::block_period();
-		let block_number = <system::Module<T>>::block_number();
-		let blocks_remaining = Self::length() - One::one() - (block_number - Self::last_length_change()) % Self::length();
+		let blocks_remaining = Self::blocks_remaining();
 		if blocks_remaining.is_zero() {
 			false
 		} else {
 			let blocks_remaining = <T::Value as As<T::BlockNumber>>::sa(blocks_remaining);
-			now + blocks_remaining * block_period > Self::current_start() + Self::ideal_session_duration() * T::Value::sa(13) / T::Value::sa(10)	// if we're 30% behind schedule then end abnormally
+			now + blocks_remaining * block_period >
+				Self::current_start() + Self::ideal_session_duration() *
+					(T::Value::sa(100) + Self::broken_percent_late()) / T::Value::sa(100)
 		}
 	}
 }
@@ -213,6 +232,7 @@ impl<T: Trait> Executable for Module<T> {
 pub struct GenesisConfig<T: Trait> {
 	pub session_length: T::BlockNumber,
 	pub validators: Vec<T::AccountId>,
+	pub broken_percent_late: T::Value,
 }
 
 #[cfg(any(feature = "std", test))]
@@ -222,6 +242,7 @@ impl<T: Trait> Default for GenesisConfig<T> {
 		GenesisConfig {
 			session_length: T::BlockNumber::sa(1000),
 			validators: vec![],
+			broken_percent_late: T::Value::sa(30),
 		}
 	}
 }
@@ -237,7 +258,8 @@ impl<T: Trait> primitives::BuildStorage for GenesisConfig<T>
 			twox_128(<SessionLength<T>>::key()).to_vec() => self.session_length.encode(),
 			twox_128(<CurrentIndex<T>>::key()).to_vec() => T::BlockNumber::sa(0).encode(),
 			twox_128(<CurrentStart<T>>::key()).to_vec() => T::Value::zero().encode(),
-			twox_128(<Validators<T>>::key()).to_vec() => self.validators.encode()
+			twox_128(<Validators<T>>::key()).to_vec() => self.validators.encode(),
+			twox_128(<BrokenPercentLate<T>>::key()).to_vec() => self.broken_percent_late.encode()
 		]
 	}
 }
@@ -280,6 +302,7 @@ mod tests {
 
 	type System = system::Module<Test>;
 	type Consensus = consensus::Module<Test>;
+	type Timestamp = timestamp::Module<Test>;
 	type Session = Module<Test>;
 
 	fn new_test_ext() -> runtime_io::TestExternalities {
@@ -288,10 +311,13 @@ mod tests {
 			code: vec![],
 			authorities: vec![1, 2, 3],
 		}.build_storage());
-		t.extend(timestamp::GenesisConfig::<Test>::default().build_storage());
+		t.extend(timestamp::GenesisConfig::<Test>{
+			period: 5,
+		}.build_storage());
 		t.extend(GenesisConfig::<Test>{
 			session_length: 2,
 			validators: vec![1, 2, 3],
+			broken_percent_late: 30,
 		}.build_storage());
 		t
 	}
@@ -305,6 +331,36 @@ mod tests {
 		});
 	}
 
+	#[test]
+	fn should_identify_broken_validation() {
+		with_externalities(&mut new_test_ext(), || {
+			System::set_block_number(2);
+			assert_eq!(Session::blocks_remaining(), 0);
+			Timestamp::set_timestamp(0);
+			assert_ok!(Session::set_length(3));
+			Session::check_rotate_session();
+			assert_eq!(Session::current_index(), 1);
+			assert_eq!(Session::length(), 3);
+			assert_eq!(Session::current_start(), 0);
+			assert_eq!(Session::ideal_session_duration(), 15);
+			// ideal end = 0 + 15 * 3 = 15
+			// broken_limit = 15 * 130 / 100 = 19
+
+			System::set_block_number(3);
+			assert_eq!(Session::blocks_remaining(), 2);
+			Timestamp::set_timestamp(9);				// earliest end = 9 + 2 * 5 = 19; OK.
+			assert!(!Session::broken_validation());
+			Session::check_rotate_session();
+
+			System::set_block_number(4);
+			assert_eq!(Session::blocks_remaining(), 1);
+			Timestamp::set_timestamp(15);				// another 1 second late. earliest end = 15 + 1 * 5 = 20; broken.
+			assert!(Session::broken_validation());
+			Session::check_rotate_session();
+			assert_eq!(Session::current_index(), 2);
+		});
+	}
+
 	#[test]
 	fn session_length_change_should_work() {
 		with_externalities(&mut new_test_ext(), || {

substrate/runtime/staking/src/mock.rs

@@ -80,6 +80,7 @@ pub fn new_test_ext(ext_deposit: u64, session_length: u64, sessions_per_era: u64
 	t.extend(session::GenesisConfig::<Test>{
 		session_length,
 		validators: vec![10, 20],
+		broken_percent_late: 30,
 	}.build_storage());
 	t.extend(GenesisConfig::<Test>{
 		sessions_per_era,

substrate/runtime/timestamp/src/lib.rs

@@ -62,7 +62,7 @@ decl_module! {
 
 decl_storage! {
 	trait Store for Module<T: Trait>;
-	pub Now get(now): b"tim:val" => required T::Value;
+	pub Now get(now): b"tim:val" => T::Value;
 	// The minimum (and advised) period between blocks.
 	pub BlockPeriod get(block_period): b"tim:block_period" => required T::Value;
 
@@ -72,7 +72,7 @@ decl_storage! {
 
 impl<T: Trait> Module<T> {
 	pub fn get() -> T::Value {
-		<Self as Store>::Now::get()
+		Self::now().expect("This must always exist apart from before the first call to `set`; `set` happens prior to anything else; qed")
 	}
 
 	/// Set the current time.
@@ -84,14 +84,22 @@ impl<T: Trait> Module<T> {
 			"Timestamp extrinsic must be at position {} in the block",
 			T::TIMESTAMP_SET_POSITION
 		);
-		assert!(
-			now >= Self::get() + Self::block_period(),
-			"Timestamp but increment by at least <BlockPeriod> between sequential blocks"
-		);
+		if let Some(t) = Self::now() {
+			assert!(
+				now >= t + Self::block_period(),
+				"Timestamp but increment by at least <BlockPeriod> between sequential blocks"
+			);
+		}
 		<Self as Store>::Now::put(now);
 		<Self as Store>::DidUpdate::put(true);
 		Ok(())
 	}
+
+	/// Set the timestamp to something in particular. Only used for tests.
+	#[cfg(any(feature = "std", test))]
+	pub fn set_timestamp(now: T::Value) {
+		<Self as Store>::Now::put(now);
+	}
 }
 
 impl<T: Trait> Executable for Module<T> {
@@ -102,15 +110,13 @@ impl<T: Trait> Executable for Module<T> {
 
 #[cfg(any(feature = "std", test))]
 pub struct GenesisConfig<T: Trait> {
-	pub now: T::Value,
 	pub period: T::Value,
 }
 
 #[cfg(any(feature = "std", test))]
 impl<T: Trait> Default for GenesisConfig<T> {
 	fn default() -> Self {
 		GenesisConfig {
-			now: T::Value::default(),
 			period: T::Value::default(),
 		}
 	}
@@ -123,7 +129,6 @@ impl<T: Trait> runtime_primitives::BuildStorage for GenesisConfig<T>
 		use runtime_io::twox_128;
 		use codec::Slicable;
 		map![
-			twox_128(<Now<T>>::key()).to_vec() => self.now.encode(),
 			twox_128(<BlockPeriod<T>>::key()).to_vec() => self.period.encode()
 		]
 	}
@@ -167,10 +172,10 @@ mod tests {
 	#[test]
 	fn timestamp_works() {
 		let mut t = system::GenesisConfig::<Test>::default().build_storage();
-		t.extend(GenesisConfig::<Test> { now: 42, period: 0 }.build_storage());
+		t.extend(GenesisConfig::<Test> { period: 0 }.build_storage());
 
 		with_externalities(&mut t, || {
-			assert_eq!(<Timestamp as Store>::Now::get(), 42);
+			Timestamp::set_timestamp(42);
 			assert_ok!(Timestamp::aux_dispatch(Call::set(69), &0));
 			assert_eq!(Timestamp::now(), 69);
 		});
@@ -180,9 +185,10 @@ mod tests {
 	#[should_panic(expected = "Timestamp must be updated only once in the block")]
 	fn double_timestamp_should_fail() {
 		let mut t = system::GenesisConfig::<Test>::default().build_storage();
-		t.extend(GenesisConfig::<Test> { now: 42, period: 5 }.build_storage());
+		t.extend(GenesisConfig::<Test> { period: 5 }.build_storage());
 
 		with_externalities(&mut t, || {
+			Timestamp::set_timestamp(42);
 			assert_ok!(Timestamp::aux_dispatch(Call::set(69), &0));
 			let _ = Timestamp::aux_dispatch(Call::set(70), &0);
 		});
@@ -195,6 +201,7 @@ mod tests {
 		t.extend(GenesisConfig::<Test> { now: 42, period: 5 }.build_storage());
 
 		with_externalities(&mut t, || {
+			Timestamp::set_timestamp(42);
 			let _ = Timestamp::aux_dispatch(Call::set(46), &0);
 		});
 	}