Remove inherent in pallet-babe (#8124)

substrate/Cargo.lock

@@ -3963,6 +3963,7 @@ dependencies = [
  "parity-scale-codec",
  "sc-executor",
  "sp-application-crypto",
+ "sp-consensus-babe",
  "sp-core",
  "sp-externalities",
  "sp-io",
@@ -4496,7 +4497,6 @@ dependencies = [
  "sp-consensus-babe",
  "sp-consensus-vrf",
  "sp-core",
- "sp-inherents",
  "sp-io",
  "sp-runtime",
  "sp-session",

substrate/bin/node/executor/Cargo.toml

@@ -39,6 +39,7 @@ pallet-timestamp = { version = "3.0.0", path = "../../../frame/timestamp" }
 pallet-transaction-payment = { version = "3.0.0", path = "../../../frame/transaction-payment" }
 pallet-treasury = { version = "3.0.0", path = "../../../frame/treasury" }
 sp-application-crypto = { version = "3.0.0", path = "../../../primitives/application-crypto" }
+sp-consensus-babe = { version = "0.9.0", path = "../../../primitives/consensus/babe" }
 sp-runtime = { version = "3.0.0", path = "../../../primitives/runtime" }
 sp-externalities = { version = "0.9.0", path = "../../../primitives/externalities" }
 substrate-test-client = { version = "2.0.0", path = "../../../test-utils/client" }

substrate/bin/node/executor/tests/basic.rs

@@ -32,7 +32,7 @@ use frame_system::{self, EventRecord, Phase};
 use node_runtime::{
 	Header, Block, UncheckedExtrinsic, CheckedExtrinsic, Call, Runtime, Balances,
 	System, TransactionPayment, Event,
-	constants::currency::*,
+	constants::{time::SLOT_DURATION, currency::*},
 };
 use node_primitives::{Balance, Hash};
 use wat;
@@ -76,6 +76,7 @@ fn set_heap_pages<E: Externalities>(ext: &mut E, heap_pages: u64) {
 }
 
 fn changes_trie_block() -> (Vec<u8>, Hash) {
+	let time = 42 * 1000;
 	construct_block(
 		&mut new_test_ext(compact_code_unwrap(), true),
 		1,
@@ -83,13 +84,14 @@ fn changes_trie_block() -> (Vec<u8>, Hash) {
 		vec![
 			CheckedExtrinsic {
 				signed: None,
-				function: Call::Timestamp(pallet_timestamp::Call::set(42 * 1000)),
+				function: Call::Timestamp(pallet_timestamp::Call::set(time)),
 			},
 			CheckedExtrinsic {
 				signed: Some((alice(), signed_extra(0, 0))),
 				function: Call::Balances(pallet_balances::Call::transfer(bob().into(), 69 * DOLLARS)),
 			},
-		]
+		],
+		(time / SLOT_DURATION).into(),
 	)
 }
 
@@ -98,6 +100,7 @@ fn changes_trie_block() -> (Vec<u8>, Hash) {
 /// from block1's execution to block2 to derive the correct storage_root.
 fn blocks() -> ((Vec<u8>, Hash), (Vec<u8>, Hash)) {
 	let mut t = new_test_ext(compact_code_unwrap(), false);
+	let time1 = 42 * 1000;
 	let block1 = construct_block(
 		&mut t,
 		1,
@@ -105,14 +108,16 @@ fn blocks() -> ((Vec<u8>, Hash), (Vec<u8>, Hash)) {
 		vec![
 			CheckedExtrinsic {
 				signed: None,
-				function: Call::Timestamp(pallet_timestamp::Call::set(42 * 1000)),
+				function: Call::Timestamp(pallet_timestamp::Call::set(time1)),
 			},
 			CheckedExtrinsic {
 				signed: Some((alice(), signed_extra(0, 0))),
 				function: Call::Balances(pallet_balances::Call::transfer(bob().into(), 69 * DOLLARS)),
 			},
-		]
+		],
+		(time1 / SLOT_DURATION).into(),
 	);
+	let time2 = 52 * 1000;
 	let block2 = construct_block(
 		&mut t,
 		2,
@@ -120,7 +125,7 @@ fn blocks() -> ((Vec<u8>, Hash), (Vec<u8>, Hash)) {
 		vec![
 			CheckedExtrinsic {
 				signed: None,
-				function: Call::Timestamp(pallet_timestamp::Call::set(52 * 1000)),
+				function: Call::Timestamp(pallet_timestamp::Call::set(time2)),
 			},
 			CheckedExtrinsic {
 				signed: Some((bob(), signed_extra(0, 0))),
@@ -130,12 +135,13 @@ fn blocks() -> ((Vec<u8>, Hash), (Vec<u8>, Hash)) {
 				signed: Some((alice(), signed_extra(1, 0))),
 				function: Call::Balances(pallet_balances::Call::transfer(bob().into(), 15 * DOLLARS)),
 			}
-		]
+		],
+		(time2 / SLOT_DURATION).into(),
 	);
 
 	// session change => consensus authorities change => authorities change digest item appears
 	let digest = Header::decode(&mut &block2.0[..]).unwrap().digest;
-	assert_eq!(digest.logs().len(), 0);
+	assert_eq!(digest.logs().len(), 1 /* Just babe slot */);
 
 	(block1, block2)
 }
@@ -154,7 +160,8 @@ fn block_with_size(time: u64, nonce: u32, size: usize) -> (Vec<u8>, Hash) {
 				signed: Some((alice(), signed_extra(nonce, 0))),
 				function: Call::System(frame_system::Call::remark(vec![0; size])),
 			}
-		]
+		],
+		(time * 1000 / SLOT_DURATION).into(),
 	)
 }
 
@@ -590,6 +597,7 @@ fn deploying_wasm_contract_should_work() {
 
 	let subsistence = pallet_contracts::Module::<Runtime>::subsistence_threshold();
 
+	let time = 42 * 1000;
 	let b = construct_block(
 		&mut new_test_ext(compact_code_unwrap(), false),
 		1,
@@ -597,7 +605,7 @@ fn deploying_wasm_contract_should_work() {
 		vec![
 			CheckedExtrinsic {
 				signed: None,
-				function: Call::Timestamp(pallet_timestamp::Call::set(42 * 1000)),
+				function: Call::Timestamp(pallet_timestamp::Call::set(time)),
 			},
 			CheckedExtrinsic {
 				signed: Some((charlie(), signed_extra(0, 0))),
@@ -622,7 +630,8 @@ fn deploying_wasm_contract_should_work() {
 					)
 				),
 			},
-		]
+		],
+		(time / SLOT_DURATION).into(),
 	);
 
 	let mut t = new_test_ext(compact_code_unwrap(), false);

substrate/bin/node/executor/tests/common.rs

@@ -19,6 +19,7 @@ use codec::{Encode, Decode};
 use frame_system::offchain::AppCrypto;
 use frame_support::Hashable;
 use sp_state_machine::TestExternalities as CoreTestExternalities;
+use sp_consensus_babe::{BABE_ENGINE_ID, Slot, digests::{PreDigest, SecondaryPlainPreDigest}};
 use sp_core::{
 	NeverNativeValue, NativeOrEncoded,
 	crypto::KeyTypeId,
@@ -29,6 +30,8 @@ use sp_runtime::{
 	ApplyExtrinsicResult,
 	MultiSigner,
 	MultiSignature,
+	Digest,
+	DigestItem,
 	traits::{Header as HeaderT, BlakeTwo256},
 };
 use sc_executor::{NativeExecutor, WasmExecutionMethod};
@@ -145,6 +148,7 @@ pub fn construct_block(
 	number: BlockNumber,
 	parent_hash: Hash,
 	extrinsics: Vec<CheckedExtrinsic>,
+	babe_slot: Slot,
 ) -> (Vec<u8>, Hash) {
 	use sp_trie::{TrieConfiguration, trie_types::Layout};
 
@@ -162,7 +166,17 @@ pub fn construct_block(
 		number,
 		extrinsics_root,
 		state_root: Default::default(),
-		digest: Default::default(),
+		digest: Digest {
+			logs: vec![
+				DigestItem::PreRuntime(
+					BABE_ENGINE_ID,
+					PreDigest::SecondaryPlain(SecondaryPlainPreDigest {
+						slot: babe_slot,
+						authority_index: 42,
+					}).encode()
+				),
+			],
+		},
 	};
 
 	// execute the block to get the real header.

substrate/bin/node/executor/tests/fees.rs

@@ -25,7 +25,7 @@ use sp_runtime::{Perbill, FixedPointNumber};
 use node_runtime::{
 	CheckedExtrinsic, Call, Runtime, Balances, TransactionPayment, Multiplier,
 	TransactionByteFee,
-	constants::currency::*,
+	constants::{time::SLOT_DURATION, currency::*},
 };
 use node_primitives::Balance;
 use node_testing::keyring::*;
@@ -46,6 +46,7 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
 
 	let mut tt = new_test_ext(compact_code_unwrap(), false);
 
+	let time1 = 42 * 1000;
 	// big one in terms of weight.
 	let block1 = construct_block(
 		&mut tt,
@@ -54,15 +55,17 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
 		vec![
 			CheckedExtrinsic {
 				signed: None,
-				function: Call::Timestamp(pallet_timestamp::Call::set(42 * 1000)),
+				function: Call::Timestamp(pallet_timestamp::Call::set(time1)),
 			},
 			CheckedExtrinsic {
 				signed: Some((charlie(), signed_extra(0, 0))),
 				function: Call::System(frame_system::Call::fill_block(Perbill::from_percent(60))),
 			}
-		]
+		],
+		(time1 / SLOT_DURATION).into(),
 	);
 
+	let time2 = 52 * 1000;
 	// small one in terms of weight.
 	let block2 = construct_block(
 		&mut tt,
@@ -71,13 +74,14 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
 		vec![
 			CheckedExtrinsic {
 				signed: None,
-				function: Call::Timestamp(pallet_timestamp::Call::set(52 * 1000)),
+				function: Call::Timestamp(pallet_timestamp::Call::set(time2)),
 			},
 			CheckedExtrinsic {
 				signed: Some((charlie(), signed_extra(1, 0))),
 				function: Call::System(frame_system::Call::remark(vec![0; 1])),
 			}
-		]
+		],
+		(time2 / SLOT_DURATION).into(),
 	);
 
 	println!(
@@ -219,7 +223,7 @@ fn block_weight_capacity_report() {
 	let mut time = 10;
 	let mut nonce: Index = 0;
 	let mut block_number = 1;
-	let mut previous_hash: Hash = GENESIS_HASH.into();
+	let mut previous_hash: node_primitives::Hash = GENESIS_HASH.into();
 
 	loop {
 		let num_transfers = block_number * factor;
@@ -238,7 +242,8 @@ fn block_weight_capacity_report() {
 			&mut tt,
 			block_number,
 			previous_hash,
-			xts
+			xts,
+			(time * 1000 / SLOT_DURATION).into(),
 		);
 
 		let len = block.0.len();
@@ -286,7 +291,7 @@ fn block_length_capacity_report() {
 	let mut time = 10;
 	let mut nonce: Index = 0;
 	let mut block_number = 1;
-	let mut previous_hash: Hash = GENESIS_HASH.into();
+	let mut previous_hash: node_primitives::Hash = GENESIS_HASH.into();
 
 	loop {
 		// NOTE: this is super slow. Can probably be improved.
@@ -303,7 +308,8 @@ fn block_length_capacity_report() {
 					signed: Some((charlie(), signed_extra(nonce, 0))),
 					function: Call::System(frame_system::Call::remark(vec![0u8; (block_number * factor) as usize])),
 				},
-			]
+			],
+			(time * 1000 / SLOT_DURATION).into(),
 		);
 
 		let len = block.0.len();

substrate/bin/node/runtime/src/lib.rs

@@ -1012,7 +1012,7 @@ construct_runtime!(
 	{
 		System: frame_system::{Module, Call, Config, Storage, Event<T>},
 		Utility: pallet_utility::{Module, Call, Event},
-		Babe: pallet_babe::{Module, Call, Storage, Config, Inherent, ValidateUnsigned},
+		Babe: pallet_babe::{Module, Call, Storage, Config, ValidateUnsigned},
 		Timestamp: pallet_timestamp::{Module, Call, Storage, Inherent},
 		Authorship: pallet_authorship::{Module, Call, Storage, Inherent},
 		Indices: pallet_indices::{Module, Call, Storage, Config<T>, Event<T>},

substrate/frame/babe/Cargo.toml

@@ -24,7 +24,6 @@ serde = { version = "1.0.101", optional = true }
 sp-application-crypto = { version = "3.0.0", default-features = false, path = "../../primitives/application-crypto" }
 sp-consensus-babe = { version = "0.9.0", default-features = false, path = "../../primitives/consensus/babe" }
 sp-consensus-vrf = { version = "0.9.0", default-features = false, path = "../../primitives/consensus/vrf" }
-sp-inherents = { version = "3.0.0", default-features = false, path = "../../primitives/inherents" }
 sp-io = { version = "3.0.0", default-features = false, path = "../../primitives/io" }
 sp-runtime = { version = "3.0.0", default-features = false, path = "../../primitives/runtime" }
 sp-session = { version = "3.0.0", default-features = false, path = "../../primitives/session" }
@@ -54,7 +53,6 @@ std = [
 	"sp-application-crypto/std",
 	"sp-consensus-babe/std",
 	"sp-consensus-vrf/std",
-	"sp-inherents/std",
 	"sp-io/std",
 	"sp-runtime/std",
 	"sp-session/std",

substrate/frame/babe/src/lib.rs

@@ -33,20 +33,18 @@ use frame_system::{ensure_none, ensure_signed};
 use sp_application_crypto::Public;
 use sp_runtime::{
 	generic::DigestItem,
-	traits::{Hash, IsMember, One, SaturatedConversion, Saturating},
+	traits::{Hash, IsMember, One, SaturatedConversion, Saturating, Zero},
 	ConsensusEngineId, KeyTypeId,
 };
 use sp_session::{GetSessionNumber, GetValidatorCount};
-use sp_std::{prelude::*, result};
+use sp_std::prelude::*;
 use sp_timestamp::OnTimestampSet;
 
 use sp_consensus_babe::{
 	digests::{NextConfigDescriptor, NextEpochDescriptor, PreDigest},
-	inherents::{BabeInherentData, INHERENT_IDENTIFIER},
 	BabeAuthorityWeight, ConsensusLog, Epoch, EquivocationProof, Slot, BABE_ENGINE_ID,
 };
 use sp_consensus_vrf::schnorrkel;
-use sp_inherents::{InherentData, InherentIdentifier, MakeFatalError, ProvideInherent};
 
 pub use sp_consensus_babe::{AuthorityId, PUBLIC_KEY_LENGTH, RANDOMNESS_LENGTH, VRF_OUTPUT_LENGTH};
 
@@ -744,7 +742,15 @@ impl<T: Config> Module<T> {
 }
 
 impl<T: Config> OnTimestampSet<T::Moment> for Module<T> {
-	fn on_timestamp_set(_moment: T::Moment) { }
+	fn on_timestamp_set(moment: T::Moment) {
+		let slot_duration = Self::slot_duration();
+		assert!(!slot_duration.is_zero(), "Babe slot duration cannot be zero.");
+
+		let timestamp_slot = moment / slot_duration;
+		let timestamp_slot = Slot::from(timestamp_slot.saturated_into::<u64>());
+
+		assert!(CurrentSlot::get() == timestamp_slot, "Timestamp slot must match `CurrentSlot`");
+	}
 }
 
 impl<T: Config> frame_support::traits::EstimateNextSessionRotation<T::BlockNumber> for Module<T> {
@@ -818,29 +824,3 @@ fn compute_randomness(
 
 	sp_io::hashing::blake2_256(&s)
 }
-
-impl<T: Config> ProvideInherent for Module<T> {
-	type Call = pallet_timestamp::Call<T>;
-	type Error = MakeFatalError<sp_inherents::Error>;
-	const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER;
-
-	fn create_inherent(_: &InherentData) -> Option<Self::Call> {
-		None
-	}
-
-	fn check_inherent(call: &Self::Call, data: &InherentData) -> result::Result<(), Self::Error> {
-		let timestamp = match call {
-			pallet_timestamp::Call::set(ref timestamp) => timestamp.clone(),
-			_ => return Ok(()),
-		};
-
-		let timestamp_based_slot = (timestamp / Self::slot_duration()).saturated_into::<u64>();
-		let seal_slot = data.babe_inherent_data()?;
-
-		if timestamp_based_slot == *seal_slot {
-			Ok(())
-		} else {
-			Err(sp_inherents::Error::from("timestamp set in block doesn't match slot in seal").into())
-		}
-	}
-}

substrate/frame/babe/src/mock.rs

@@ -54,7 +54,7 @@ frame_support::construct_runtime!(
 		Balances: pallet_balances::{Module, Call, Storage, Config<T>, Event<T>},
 		Historical: pallet_session_historical::{Module},
 		Offences: pallet_offences::{Module, Call, Storage, Event},
-		Babe: pallet_babe::{Module, Call, Storage, Config, Inherent, ValidateUnsigned},
+		Babe: pallet_babe::{Module, Call, Storage, Config, ValidateUnsigned},
 		Staking: pallet_staking::{Module, Call, Storage, Config<T>, Event<T>},
 		Session: pallet_session::{Module, Call, Storage, Event, Config<T>},
 		Timestamp: pallet_timestamp::{Module, Call, Storage, Inherent},

substrate/primitives/consensus/babe/src/inherents.rs

@@ -52,6 +52,7 @@ impl BabeInherentData for InherentData {
 }
 
 /// Provides the slot duration inherent data for BABE.
+// TODO: Remove in the future. https://github.com/paritytech/substrate/issues/8029
 #[cfg(feature = "std")]
 pub struct InherentDataProvider {
 	slot_duration: u64,