Add ChainTime associated type (#410)

* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

bridges/bin/rialto/runtime/src/kovan.rs

@@ -21,7 +21,8 @@ use bp_header_chain::BaseHeaderChain;
 use frame_support::RuntimeDebug;
 use hex_literal::hex;
 use pallet_bridge_eth_poa::{
-	AuraConfiguration, PruningStrategy as BridgePruningStrategy, ValidatorsConfiguration, ValidatorsSource,
+	AuraConfiguration, ChainTime as TChainTime, PruningStrategy as BridgePruningStrategy, ValidatorsConfiguration,
+	ValidatorsSource,
 };
 use sp_std::prelude::*;
 
@@ -134,6 +135,17 @@ impl BridgePruningStrategy for PruningStrategy {
 	}
 }
 
+/// PoA Header timestamp verification against `Timestamp` pallet.
+#[derive(Default, RuntimeDebug)]
+pub struct ChainTime;
+
+impl TChainTime for ChainTime {
+	fn is_timestamp_ahead(&self, timestamp: u64) -> bool {
+		let now = super::Timestamp::now();
+		timestamp > now
+	}
+}
+
 /// The Kovan Blockchain as seen by the runtime.
 pub struct KovanBlockchain;
 

bridges/bin/rialto/runtime/src/lib.rs

@@ -234,6 +234,7 @@ impl pallet_bridge_eth_poa::Trait<RialtoPoA> for Runtime {
 	type FinalityVotesCachingInterval = rialto_poa::FinalityVotesCachingInterval;
 	type ValidatorsConfiguration = rialto_poa::BridgeValidatorsConfiguration;
 	type PruningStrategy = rialto_poa::PruningStrategy;
+	type ChainTime = rialto_poa::ChainTime;
 	type OnHeadersSubmitted = ();
 }
 
@@ -243,6 +244,7 @@ impl pallet_bridge_eth_poa::Trait<Kovan> for Runtime {
 	type FinalityVotesCachingInterval = kovan::FinalityVotesCachingInterval;
 	type ValidatorsConfiguration = kovan::BridgeValidatorsConfiguration;
 	type PruningStrategy = kovan::PruningStrategy;
+	type ChainTime = kovan::ChainTime;
 	type OnHeadersSubmitted = ();
 }
 

bridges/bin/rialto/runtime/src/rialto_poa.rs

@@ -23,7 +23,8 @@ use bp_header_chain::BaseHeaderChain;
 use frame_support::RuntimeDebug;
 use hex_literal::hex;
 use pallet_bridge_eth_poa::{
-	AuraConfiguration, PruningStrategy as TPruningStrategy, ValidatorsConfiguration, ValidatorsSource,
+	AuraConfiguration, ChainTime as TChainTime, PruningStrategy as TPruningStrategy, ValidatorsConfiguration,
+	ValidatorsSource,
 };
 use sp_std::prelude::*;
 
@@ -109,6 +110,17 @@ impl TPruningStrategy for PruningStrategy {
 	}
 }
 
+/// ChainTime provider
+#[derive(Default)]
+pub struct ChainTime;
+
+impl TChainTime for ChainTime {
+	fn is_timestamp_ahead(&self, timestamp: u64) -> bool {
+		let now = super::Timestamp::now();
+		timestamp > now
+	}
+}
+
 /// The Rialto PoA Blockchain as seen by the runtime.
 pub struct RialtoBlockchain;
 

bridges/modules/ethereum/Cargo.toml

@@ -26,6 +26,7 @@ sp-std = { version = "2.0", default-features = false }
 
 [dev-dependencies]
 libsecp256k1 = { version = "0.3.4", features = ["hmac"] }
+hex-literal = "0.3"
 
 [features]
 default = ["std"]

bridges/modules/ethereum/src/error.rs

@@ -62,6 +62,8 @@ pub enum Error {
 	UnsignedTooFarInTheFuture = 19,
 	/// Trying to finalize sibling of finalized block.
 	TryingToFinalizeSibling = 20,
+	/// Header timestamp is ahead of on-chain timestamp
+	HeaderTimestampIsAhead = 21,
 }
 
 impl Error {
@@ -88,6 +90,7 @@ impl Error {
 			Error::TransactionsReceiptsMismatch => "Invalid transactions receipts provided",
 			Error::UnsignedTooFarInTheFuture => "The unsigned header is too far in future",
 			Error::TryingToFinalizeSibling => "Trying to finalize sibling of finalized block",
+			Error::HeaderTimestampIsAhead => "Header timestamp is ahead of on-chain timestamp",
 		}
 	}
 

bridges/modules/ethereum/src/import.rs

@@ -18,7 +18,7 @@ use crate::error::Error;
 use crate::finality::finalize_blocks;
 use crate::validators::{Validators, ValidatorsConfiguration};
 use crate::verification::{is_importable_header, verify_aura_header};
-use crate::{AuraConfiguration, ChangeToEnact, PruningStrategy, Storage};
+use crate::{AuraConfiguration, ChainTime, ChangeToEnact, PruningStrategy, Storage};
 use bp_eth_poa::{AuraHeader, HeaderId, Receipt};
 use sp_std::{collections::btree_map::BTreeMap, prelude::*};
 
@@ -31,13 +31,16 @@ use sp_std::{collections::btree_map::BTreeMap, prelude::*};
 /// we have NOT imported.
 /// Returns error if fatal error has occured during import. Some valid headers may be
 /// imported in this case.
-pub fn import_headers<S: Storage, PS: PruningStrategy>(
+/// TODO: update me (https://github.com/paritytech/parity-bridges-common/issues/415)
+#[allow(clippy::too_many_arguments)]
+pub fn import_headers<S: Storage, PS: PruningStrategy, CT: ChainTime>(
 	storage: &mut S,
 	pruning_strategy: &mut PS,
 	aura_config: &AuraConfiguration,
 	validators_config: &ValidatorsConfiguration,
 	submitter: Option<S::Submitter>,
 	headers: Vec<(AuraHeader, Option<Vec<Receipt>>)>,
+	chain_time: &CT,
 	finalized_headers: &mut BTreeMap<S::Submitter, u64>,
 ) -> Result<(u64, u64), Error> {
 	let mut useful = 0;
@@ -50,6 +53,7 @@ pub fn import_headers<S: Storage, PS: PruningStrategy>(
 			validators_config,
 			submitter.clone(),
 			header,
+			chain_time,
 			receipts,
 		);
 
@@ -79,20 +83,23 @@ pub type FinalizedHeaders<S> = Vec<(HeaderId, Option<<S as Storage>::Submitter>)
 /// has returned true.
 ///
 /// Returns imported block id and list of all finalized headers.
-pub fn import_header<S: Storage, PS: PruningStrategy>(
+/// TODO: update me (https://github.com/paritytech/parity-bridges-common/issues/415)
+#[allow(clippy::too_many_arguments)]
+pub fn import_header<S: Storage, PS: PruningStrategy, CT: ChainTime>(
 	storage: &mut S,
 	pruning_strategy: &mut PS,
 	aura_config: &AuraConfiguration,
 	validators_config: &ValidatorsConfiguration,
 	submitter: Option<S::Submitter>,
 	header: AuraHeader,
+	chain_time: &CT,
 	receipts: Option<Vec<Receipt>>,
 ) -> Result<(HeaderId, FinalizedHeaders<S>), Error> {
 	// first check that we are able to import this header at all
 	let (header_id, finalized_id) = is_importable_header(storage, &header)?;
 
 	// verify header
-	let import_context = verify_aura_header(storage, aura_config, submitter, &header)?;
+	let import_context = verify_aura_header(storage, aura_config, submitter, &header, chain_time)?;
 
 	// check if block schedules new validators
 	let validators = Validators::new(validators_config);
@@ -195,6 +202,7 @@ mod tests {
 					&test_validators_config(),
 					None,
 					Default::default(),
+					&(),
 					None,
 				),
 				Err(Error::AncientHeader),
@@ -215,6 +223,7 @@ mod tests {
 					&test_validators_config(),
 					None,
 					header.clone(),
+					&(),
 					None,
 				)
 				.map(|_| ()),
@@ -228,6 +237,7 @@ mod tests {
 					&test_validators_config(),
 					None,
 					header,
+					&(),
 					None,
 				)
 				.map(|_| ()),
@@ -254,6 +264,7 @@ mod tests {
 					&validators_config,
 					None,
 					header,
+					&(),
 					None
 				)
 				.map(|_| ()),
@@ -291,6 +302,7 @@ mod tests {
 					&validators_config,
 					Some(100),
 					header,
+					&(),
 					None,
 				)
 				.unwrap();
@@ -320,6 +332,7 @@ mod tests {
 				&validators_config,
 				Some(101),
 				header11.clone(),
+				&(),
 				Some(vec![validators_change_receipt(latest_block_id.hash)]),
 			)
 			.unwrap();
@@ -345,6 +358,7 @@ mod tests {
 					&validators_config,
 					Some(102),
 					header,
+					&(),
 					None,
 				)
 				.unwrap();
@@ -374,6 +388,7 @@ mod tests {
 				&validators_config,
 				Some(103),
 				header,
+				&(),
 				None,
 			)
 			.unwrap();
@@ -404,6 +419,7 @@ mod tests {
 			)),
 			None,
 			header,
+			&(),
 			None,
 		)
 		.map(|_| id)

bridges/modules/ethereum/src/lib.rs

@@ -326,6 +326,25 @@ pub trait PruningStrategy: Default {
 	fn pruning_upper_bound(&mut self, best_number: u64, best_finalized_number: u64) -> u64;
 }
 
+/// ChainTime represents the runtime on-chain time
+pub trait ChainTime: Default {
+	/// Is a header timestamp ahead of the current on-chain time.
+	///
+	/// Check whether `timestamp` is ahead (i.e greater than) the current on-chain
+	/// time. If so, return `true`, `false` otherwise.
+	fn is_timestamp_ahead(&self, timestamp: u64) -> bool;
+}
+
+/// ChainTime implementation for the empty type.
+///
+/// This implementation will allow a runtime without the timestamp pallet to use
+/// the empty type as its ChainTime associated type.
+impl ChainTime for () {
+	fn is_timestamp_ahead(&self, _: u64) -> bool {
+		false
+	}
+}
+
 /// Callbacks for header submission rewards/penalties.
 pub trait OnHeadersSubmitted<AccountId> {
 	/// Called when valid headers have been submitted.
@@ -366,6 +385,8 @@ pub trait Trait<I = DefaultInstance>: frame_system::Trait {
 	type FinalityVotesCachingInterval: Get<Option<u64>>;
 	/// Headers pruning strategy.
 	type PruningStrategy: PruningStrategy;
+	/// Header timestamp verification against current on-chain time.
+	type ChainTime: ChainTime;
 
 	/// Handler for headers submission result.
 	type OnHeadersSubmitted: OnHeadersSubmitted<Self::AccountId>;
@@ -385,6 +406,7 @@ decl_module! {
 				&T::ValidatorsConfiguration::get(),
 				None,
 				header,
+				&T::ChainTime::default(),
 				receipts,
 			).map_err(|e| e.msg())?;
 		}
@@ -406,6 +428,7 @@ decl_module! {
 				&T::ValidatorsConfiguration::get(),
 				Some(submitter.clone()),
 				headers_with_receipts,
+				&T::ChainTime::default(),
 				&mut finalized_headers,
 			);
 
@@ -531,6 +554,7 @@ impl<T: Trait<I>, I: Instance> frame_support::unsigned::ValidateUnsigned for Mod
 					&T::ValidatorsConfiguration::get(),
 					&pool_configuration(),
 					header,
+					&T::ChainTime::default(),
 					receipts.as_ref(),
 				);
 

bridges/modules/ethereum/src/mock.rs

@@ -18,7 +18,7 @@ pub use crate::test_utils::{insert_header, validator_utils::*, validators_change
 pub use bp_eth_poa::signatures::secret_to_address;
 
 use crate::validators::{ValidatorsConfiguration, ValidatorsSource};
-use crate::{AuraConfiguration, GenesisConfig, PruningStrategy, Trait};
+use crate::{AuraConfiguration, ChainTime, GenesisConfig, PruningStrategy, Trait};
 use bp_eth_poa::{Address, AuraHeader, H256, U256};
 use frame_support::{impl_outer_origin, parameter_types, weights::Weight};
 use secp256k1::SecretKey;
@@ -83,6 +83,7 @@ impl Trait for TestRuntime {
 	type ValidatorsConfiguration = TestValidatorsConfiguration;
 	type FinalityVotesCachingInterval = TestFinalityVotesCachingInterval;
 	type PruningStrategy = KeepSomeHeadersBehindBest;
+	type ChainTime = ConstChainTime;
 	type OnHeadersSubmitted = ();
 }
 
@@ -168,3 +169,14 @@ impl PruningStrategy for KeepSomeHeadersBehindBest {
 		best_number.saturating_sub(self.0)
 	}
 }
+
+/// Constant chain time
+#[derive(Default)]
+pub struct ConstChainTime;
+
+impl ChainTime for ConstChainTime {
+	fn is_timestamp_ahead(&self, timestamp: u64) -> bool {
+		let now = i32::max_value() as u64 / 2;
+		timestamp > now
+	}
+}

bridges/modules/ethereum/src/verification.rs

@@ -16,7 +16,7 @@
 
 use crate::error::Error;
 use crate::validators::{Validators, ValidatorsConfiguration};
-use crate::{AuraConfiguration, ImportContext, PoolConfiguration, ScheduledChange, Storage};
+use crate::{AuraConfiguration, ChainTime, ImportContext, PoolConfiguration, ScheduledChange, Storage};
 use bp_eth_poa::{
 	public_to_address, step_validator, Address, AuraHeader, HeaderId, Receipt, SealedEmptyStep, H256, H520, U128, U256,
 };
@@ -46,19 +46,20 @@ pub fn is_importable_header<S: Storage>(storage: &S, header: &AuraHeader) -> Res
 /// Try accept unsigned aura header into transaction pool.
 ///
 /// Returns required and provided tags.
-pub fn accept_aura_header_into_pool<S: Storage>(
+pub fn accept_aura_header_into_pool<S: Storage, CT: ChainTime>(
 	storage: &S,
 	config: &AuraConfiguration,
 	validators_config: &ValidatorsConfiguration,
 	pool_config: &PoolConfiguration,
 	header: &AuraHeader,
+	chain_time: &CT,
 	receipts: Option<&Vec<Receipt>>,
 ) -> Result<(Vec<TransactionTag>, Vec<TransactionTag>), Error> {
 	// check if we can verify further
 	let (header_id, _) = is_importable_header(storage, header)?;
 
 	// we can always do contextless checks
-	contextless_checks(config, header)?;
+	contextless_checks(config, header, chain_time)?;
 
 	// we want to avoid having same headers twice in the pool
 	// => we're strict about receipts here - if we need them, we require receipts to be Some,
@@ -153,14 +154,15 @@ pub fn accept_aura_header_into_pool<S: Storage>(
 }
 
 /// Verify header by Aura rules.
-pub fn verify_aura_header<S: Storage>(
+pub fn verify_aura_header<S: Storage, CT: ChainTime>(
 	storage: &S,
 	config: &AuraConfiguration,
 	submitter: Option<S::Submitter>,
 	header: &AuraHeader,
+	chain_time: &CT,
 ) -> Result<ImportContext<S::Submitter>, Error> {
 	// let's do the lightest check first
-	contextless_checks(config, header)?;
+	contextless_checks(config, header, chain_time)?;
 
 	// the rest of checks requires access to the parent header
 	let context = storage.import_context(submitter, &header.parent_hash).ok_or_else(|| {
@@ -180,7 +182,11 @@ pub fn verify_aura_header<S: Storage>(
 }
 
 /// Perform basic checks that only require header itself.
-fn contextless_checks(config: &AuraConfiguration, header: &AuraHeader) -> Result<(), Error> {
+fn contextless_checks<CT: ChainTime>(
+	config: &AuraConfiguration,
+	header: &AuraHeader,
+	chain_time: &CT,
+) -> Result<(), Error> {
 	let expected_seal_fields = expected_header_seal_fields(config, header);
 	if header.seal.len() != expected_seal_fields {
 		return Err(Error::InvalidSealArity);
@@ -207,6 +213,10 @@ fn contextless_checks(config: &AuraConfiguration, header: &AuraHeader) -> Result
 		return Err(Error::TimestampOverflow);
 	}
 
+	if chain_time.is_timestamp_ahead(header.timestamp) {
+		return Err(Error::HeaderTimestampIsAhead);
+	}
+
 	Ok(())
 }
 
@@ -358,7 +368,7 @@ mod tests {
 	use super::*;
 	use crate::mock::{
 		insert_header, run_test_with_genesis, test_aura_config, validator, validator_address, validators_addresses,
-		validators_change_receipt, AccountId, HeaderBuilder, TestRuntime, GAS_LIMIT,
+		validators_change_receipt, AccountId, ConstChainTime, HeaderBuilder, TestRuntime, GAS_LIMIT,
 	};
 	use crate::validators::ValidatorsSource;
 	use crate::DefaultInstance;
@@ -366,8 +376,9 @@ mod tests {
 		pool_configuration, BridgeStorage, FinalizedBlock, Headers, HeadersByNumber, NextValidatorsSetId,
 		ScheduledChanges, ValidatorsSet, ValidatorsSets,
 	};
-	use bp_eth_poa::{compute_merkle_root, rlp_encode, TransactionOutcome, H520};
+	use bp_eth_poa::{compute_merkle_root, rlp_encode, TransactionOutcome, H520, U256};
 	use frame_support::{StorageMap, StorageValue};
+	use hex_literal::hex;
 	use secp256k1::SecretKey;
 	use sp_runtime::transaction_validity::TransactionTag;
 
@@ -381,7 +392,7 @@ mod tests {
 	fn verify_with_config(config: &AuraConfiguration, header: &AuraHeader) -> Result<ImportContext<AccountId>, Error> {
 		run_test_with_genesis(genesis(), TOTAL_VALIDATORS, |_| {
 			let storage = BridgeStorage::<TestRuntime>::new();
-			verify_aura_header(&storage, &config, None, header)
+			verify_aura_header(&storage, &config, None, header, &ConstChainTime::default())
 		})
 	}
 
@@ -414,6 +425,7 @@ mod tests {
 				&validators_config,
 				&pool_configuration(),
 				&header,
+				&(),
 				receipts.as_ref(),
 			)
 		})
@@ -554,6 +566,50 @@ mod tests {
 		assert_ne!(default_verify(&header), Err(Error::TimestampOverflow));
 	}
 
+	#[test]
+	fn verifies_chain_time() {
+		// expected import context after verification
+		let expect = ImportContext::<AccountId> {
+			submitter: None,
+			parent_hash: hex!("6e41bff05578fc1db17f6816117969b07d2217f1f9039d8116a82764335991d3").into(),
+			parent_header: genesis(),
+			parent_total_difficulty: U256::zero(),
+			parent_scheduled_change: None,
+			validators_set_id: 0,
+			validators_set: ValidatorsSet {
+				validators: vec![
+					hex!("dc5b20847f43d67928f49cd4f85d696b5a7617b5").into(),
+					hex!("897df33a7b3c62ade01e22c13d48f98124b4480f").into(),
+					hex!("05c987b34c6ef74e0c7e69c6e641120c24164c2d").into(),
+				],
+				signal_block: None,
+				enact_block: HeaderId {
+					number: 0,
+					hash: hex!("6e41bff05578fc1db17f6816117969b07d2217f1f9039d8116a82764335991d3").into(),
+				},
+			},
+			last_signal_block: None,
+		};
+
+		// header is behind
+		let header = HeaderBuilder::with_parent(&genesis())
+			.timestamp(i32::max_value() as u64 / 2 - 100)
+			.sign_by(&validator(1));
+		assert_eq!(default_verify(&header).unwrap(), expect);
+
+		// header is ahead
+		let header = HeaderBuilder::with_parent(&genesis())
+			.timestamp(i32::max_value() as u64 / 2 + 100)
+			.sign_by(&validator(1));
+		assert_eq!(default_verify(&header), Err(Error::HeaderTimestampIsAhead));
+
+		// header has same timestamp as ConstChainTime
+		let header = HeaderBuilder::with_parent(&genesis())
+			.timestamp(i32::max_value() as u64 / 2)
+			.sign_by(&validator(1));
+		assert_eq!(default_verify(&header).unwrap(), expect);
+	}
+
 	#[test]
 	fn verifies_parent_existence() {
 		// when there's no parent in the storage