From fffb7a691459dcc23f859e96205a10f74bdfa07f Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 11 Nov 2022 05:42:13 -0500 Subject: [PATCH] Separate the block processing time from the latency --- substrate/node/src/service.rs | 9 +++++-- substrate/runtime/src/lib.rs | 7 +++--- .../tendermint/client/src/authority/mod.rs | 8 +++--- substrate/tendermint/client/src/lib.rs | 9 ++++--- substrate/tendermint/machine/src/ext.rs | 12 +++++++-- substrate/tendermint/machine/src/lib.rs | 25 +++++++++---------- substrate/tendermint/machine/tests/ext.rs | 7 +++--- 7 files changed, 46 insertions(+), 31 deletions(-) diff --git a/substrate/node/src/service.rs b/substrate/node/src/service.rs index 7d35507d..43646965 100644 --- a/substrate/node/src/service.rs +++ b/substrate/node/src/service.rs @@ -18,7 +18,7 @@ pub(crate) use sc_tendermint::{ TendermintClientMinimal, TendermintValidator, TendermintImport, TendermintAuthority, TendermintSelectChain, import_queue, }; -use serai_runtime::{self, MILLISECS_PER_BLOCK, opaque::Block, RuntimeApi}; +use serai_runtime::{self, TARGET_BLOCK_TIME, opaque::Block, RuntimeApi}; type FullBackend = sc_service::TFullBackend; pub type FullClient = TFullClient>; @@ -63,7 +63,10 @@ impl CreateInherentDataProviders for Cidp { pub struct TendermintValidatorFirm; impl TendermintClientMinimal for TendermintValidatorFirm { - const BLOCK_TIME_IN_SECONDS: u32 = { (MILLISECS_PER_BLOCK / 1000) as u32 }; + // 3 seconds + const BLOCK_PROCESSING_TIME_IN_SECONDS: u32 = { (TARGET_BLOCK_TIME / 2 / 1000) as u32 }; + // 1 second + const LATENCY_TIME_IN_SECONDS: u32 = { (TARGET_BLOCK_TIME / 2 / 3 / 1000) as u32 }; type Block = Block; type Backend = sc_client_db::Backend; @@ -86,6 +89,8 @@ impl TendermintValidator for TendermintValidatorFirm { pub fn new_partial( config: &Configuration, ) -> Result<(TendermintImport, PartialComponents), ServiceError> { + debug_assert_eq!(TARGET_BLOCK_TIME, 6000); + if config.keystore_remote.is_some() { return Err(ServiceError::Other("Remote Keystores are not supported".to_string())); } diff --git a/substrate/runtime/src/lib.rs b/substrate/runtime/src/lib.rs index 7e8b29f1..64661d8b 100644 --- a/substrate/runtime/src/lib.rs +++ b/substrate/runtime/src/lib.rs @@ -79,11 +79,10 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { state_version: 1, }; -pub const MILLISECS_PER_BLOCK: u64 = 6000; -pub const SLOT_DURATION: u64 = MILLISECS_PER_BLOCK; +pub const TARGET_BLOCK_TIME: u64 = 6000; /// Measured in blocks. -pub const MINUTES: BlockNumber = 60_000 / (MILLISECS_PER_BLOCK as BlockNumber); +pub const MINUTES: BlockNumber = 60_000 / (TARGET_BLOCK_TIME as BlockNumber); pub const HOURS: BlockNumber = MINUTES * 60; pub const DAYS: BlockNumber = HOURS * 24; @@ -164,7 +163,7 @@ impl pallet_randomness_collective_flip::Config for Runtime {} impl pallet_timestamp::Config for Runtime { type Moment = u64; type OnTimestampSet = (); - type MinimumPeriod = ConstU64<{ SLOT_DURATION / 2 }>; + type MinimumPeriod = ConstU64<{ TARGET_BLOCK_TIME / 2 }>; type WeightInfo = (); } diff --git a/substrate/tendermint/client/src/authority/mod.rs b/substrate/tendermint/client/src/authority/mod.rs index 2d579e9c..957bb655 100644 --- a/substrate/tendermint/client/src/authority/mod.rs +++ b/substrate/tendermint/client/src/authority/mod.rs @@ -130,8 +130,9 @@ impl TendermintAuthority { .propose( self.import.inherent_data(parent).await, Digest::default(), - // TODO: Production time, size limit - Duration::from_secs(1), + // Assumes a block cannot take longer to download than it'll take to process + Duration::from_secs((T::BLOCK_PROCESSING_TIME_IN_SECONDS / 2).into()), + // TODO: Size limit None, ) .await @@ -253,7 +254,8 @@ impl Network for TendermintAuthority { type Weights = TendermintValidators; type Block = T::Block; - const BLOCK_TIME: u32 = T::BLOCK_TIME_IN_SECONDS; + const BLOCK_PROCESSING_TIME: u32 = T::BLOCK_PROCESSING_TIME_IN_SECONDS; + const LATENCY_TIME: u32 = T::LATENCY_TIME_IN_SECONDS; fn signer(&self) -> TendermintSigner { self.active.as_ref().unwrap().signer.clone() diff --git a/substrate/tendermint/client/src/lib.rs b/substrate/tendermint/client/src/lib.rs index 7c3e96c4..880b27a4 100644 --- a/substrate/tendermint/client/src/lib.rs +++ b/substrate/tendermint/client/src/lib.rs @@ -53,7 +53,8 @@ pub fn set_config(protocol: ProtocolName) -> NonDefaultSetConfig { /// Trait consolidating all generics required by sc_tendermint for processing. pub trait TendermintClient: Send + Sync + 'static { - const BLOCK_TIME_IN_SECONDS: u32; + const BLOCK_PROCESSING_TIME_IN_SECONDS: u32; + const LATENCY_TIME_IN_SECONDS: u32; type Block: Block; type Backend: Backend + 'static; @@ -81,7 +82,8 @@ pub trait TendermintClient: Send + Sync + 'static { /// Trait implementable on firm types to automatically provide a full TendermintClient impl. pub trait TendermintClientMinimal: Send + Sync + 'static { - const BLOCK_TIME_IN_SECONDS: u32; + const BLOCK_PROCESSING_TIME_IN_SECONDS: u32; + const LATENCY_TIME_IN_SECONDS: u32; type Block: Block; type Backend: Backend + 'static; @@ -102,7 +104,8 @@ where BlockBuilderApi + TendermintApi, TransactionFor: Send + Sync + 'static, { - const BLOCK_TIME_IN_SECONDS: u32 = T::BLOCK_TIME_IN_SECONDS; + const BLOCK_PROCESSING_TIME_IN_SECONDS: u32 = T::BLOCK_PROCESSING_TIME_IN_SECONDS; + const LATENCY_TIME_IN_SECONDS: u32 = T::LATENCY_TIME_IN_SECONDS; type Block = T::Block; type Backend = T::Backend; diff --git a/substrate/tendermint/machine/src/ext.rs b/substrate/tendermint/machine/src/ext.rs index 9852f528..2858eea9 100644 --- a/substrate/tendermint/machine/src/ext.rs +++ b/substrate/tendermint/machine/src/ext.rs @@ -207,8 +207,16 @@ pub trait Network: Send + Sync { /// Type used for ordered blocks of information. type Block: Block; - // Block time in seconds - const BLOCK_TIME: u32; + /// Maximum block processing time in seconds. This should include both the actual processing time + /// and the time to download the block. + const BLOCK_PROCESSING_TIME: u32; + /// Network latency time in seconds. + const LATENCY_TIME: u32; + + /// The block time is defined as the processing time plus three times the latency. + fn block_time() -> u32 { + Self::BLOCK_PROCESSING_TIME + (3 * Self::LATENCY_TIME) + } /// Return a handle on the signer in use, usable for the entire lifetime of the machine. fn signer(&self) -> ::Signer; diff --git a/substrate/tendermint/machine/src/lib.rs b/substrate/tendermint/machine/src/lib.rs index 9dd6e4d8..ef2e580d 100644 --- a/substrate/tendermint/machine/src/lib.rs +++ b/substrate/tendermint/machine/src/lib.rs @@ -155,23 +155,22 @@ impl TendermintMachine { fn canonical_end_time(&self, round: Round) -> u64 { let mut time = self.canonical_start_time; for r in 0 .. u64::from(round.0 + 1) { - time += (r + 1) * u64::from(N::BLOCK_TIME); + time += (r + 1) * u64::from(N::block_time()); } time } fn timeout(&self, step: Step) -> Instant { - let mut round_time = Duration::from_secs(N::BLOCK_TIME.into()); - round_time *= self.round.0 + 1; - // TODO: Non-uniform timeouts. Proposal has to validate the block which will take much longer - // than any other step - let step_time = round_time / 3; - - let offset = match step { - Step::Propose => step_time, - Step::Prevote => step_time * 2, - Step::Precommit => step_time * 3, - }; + let adjusted_block = N::BLOCK_PROCESSING_TIME * (self.round.0 + 1); + let adjusted_latency = N::LATENCY_TIME * (self.round.0 + 1); + let offset = Duration::from_secs( + (match step { + Step::Propose => adjusted_block + adjusted_latency, + Step::Prevote => adjusted_block + (2 * adjusted_latency), + Step::Precommit => adjusted_block + (3 * adjusted_latency), + }) + .into(), + ); self.start_time + offset } @@ -301,7 +300,7 @@ impl TendermintMachine { // Using the genesis time in place will cause this block to be created immediately // after it, without the standard amount of separation (so their times will be // equivalent or minimally offset) - // For callers wishing to avoid this, they should pass (0, GENESIS + BLOCK_TIME) + // For callers wishing to avoid this, they should pass (0, GENESIS + N::block_time()) start_time: last_time, personal_proposal: proposal, diff --git a/substrate/tendermint/machine/tests/ext.rs b/substrate/tendermint/machine/tests/ext.rs index 2f7cd666..79a04496 100644 --- a/substrate/tendermint/machine/tests/ext.rs +++ b/substrate/tendermint/machine/tests/ext.rs @@ -103,7 +103,8 @@ impl Network for TestNetwork { type Weights = TestWeights; type Block = TestBlock; - const BLOCK_TIME: u32 = 1; + const BLOCK_PROCESSING_TIME: u32 = 2; + const LATENCY_TIME: u32 = 1; fn signer(&self) -> TestSigner { TestSigner(self.0) @@ -168,7 +169,5 @@ impl TestNetwork { #[tokio::test] async fn test() { TestNetwork::new(4).await; - for _ in 0 .. 10 { - sleep(Duration::from_secs(1)).await; - } + sleep(Duration::from_secs(30)).await; }