From 9177735ca86f441de4d44c244d4d87edec8d91d2 Mon Sep 17 00:00:00 2001 From: Tomos Wootton Date: Thu, 12 Dec 2024 14:05:48 +0000 Subject: [PATCH 1/3] feat: remove has_txn and exponential timeout check from block proposal timeout --- zilliqa/src/consensus.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/zilliqa/src/consensus.rs b/zilliqa/src/consensus.rs index 007d866d2..faf5a81a5 100644 --- a/zilliqa/src/consensus.rs +++ b/zilliqa/src/consensus.rs @@ -493,11 +493,8 @@ impl Consensus { return Ok(None); } - let ( - time_since_last_view_change, - exponential_backoff_timeout, - minimum_time_left_for_empty_block, - ) = self.get_consensus_timeout_params()?; + let (time_since_last_view_change, exponential_backoff_timeout, _) = + self.get_consensus_timeout_params(); trace!( "timeout reached create_next_block_on_timeout: {}", @@ -507,15 +504,8 @@ impl Consensus { let empty_block_timeout_ms = self.config.consensus.empty_block_timeout.as_millis() as u64; - let has_txns_for_next_block = self.transaction_pool.has_txn_ready(); - - // Check if enough time elapsed or there's something in mempool or we don't have enough - // time but let's try at least until new view can happen - if time_since_last_view_change > empty_block_timeout_ms - || has_txns_for_next_block - || (time_since_last_view_change + minimum_time_left_for_empty_block - >= exponential_backoff_timeout) - { + // Check if enough time elapsed to propse block + if time_since_last_view_change > empty_block_timeout_ms { if let Ok(Some((block, transactions))) = self.propose_new_block() { self.create_next_block_on_timeout = false; return Ok(Some(( @@ -536,7 +526,6 @@ impl Consensus { // Now consider whether we want to timeout - the timeout duration doubles every time, so it // Should eventually have all nodes on the same view - if time_since_last_view_change < exponential_backoff_timeout { trace!( "Not proceeding with view change. Current view: {} - time since last: {}, timeout requires: {}", From b9cdcfe31f1b70e5374e1368034637af6fc03bfa Mon Sep 17 00:00:00 2001 From: Tomos Wootton Date: Thu, 12 Dec 2024 15:42:22 +0000 Subject: [PATCH 2/3] feat: propose blocks as soon as empty_block_timeout is reached --- z2/src/setup.rs | 1 - zilliqa/src/cfg.rs | 4 -- zilliqa/src/consensus.rs | 67 +++++++++++---------------------- zilliqa/tests/it/main.rs | 2 - zilliqa/tests/it/persistence.rs | 1 - 5 files changed, 23 insertions(+), 52 deletions(-) diff --git a/z2/src/setup.rs b/z2/src/setup.rs index 31e32dc41..397809afe 100644 --- a/z2/src/setup.rs +++ b/z2/src/setup.rs @@ -521,7 +521,6 @@ impl Setup { scilla_address: scilla_address_default(), scilla_stdlib_dir: scilla_stdlib_dir_default(), scilla_ext_libs_path: scilla_ext_libs_path_default(), - minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(), main_shard_id: None, local_address: local_address_default(), consensus_timeout: consensus_timeout_default(), diff --git a/zilliqa/src/cfg.rs b/zilliqa/src/cfg.rs index fc29a2a5d..5c618af6a 100644 --- a/zilliqa/src/cfg.rs +++ b/zilliqa/src/cfg.rs @@ -292,10 +292,6 @@ pub struct ConsensusConfig { /// Minimum time to wait for consensus to propose new block if there are no transactions. This therefore acts also as the minimum block time. #[serde(default = "empty_block_timeout_default")] pub empty_block_timeout: Duration, - /// Minimum remaining time before end of round in which Proposer has the opportunity to broadcast empty block proposal. - /// If there is less time than this value left in a round then the view will likely move on before a proposal has time to be finalised. - #[serde(default = "minimum_time_left_for_empty_block_default")] - pub minimum_time_left_for_empty_block: Duration, /// Address of the Scilla server. Defaults to "http://localhost:3000". #[serde(default = "scilla_address_default")] pub scilla_address: String, diff --git a/zilliqa/src/consensus.rs b/zilliqa/src/consensus.rs index faf5a81a5..2efa9f450 100644 --- a/zilliqa/src/consensus.rs +++ b/zilliqa/src/consensus.rs @@ -493,7 +493,7 @@ impl Consensus { return Ok(None); } - let (time_since_last_view_change, exponential_backoff_timeout, _) = + let (time_since_last_view_change, exponential_backoff_timeout, empty_block_timeout) = self.get_consensus_timeout_params(); trace!( @@ -501,11 +501,8 @@ impl Consensus { self.create_next_block_on_timeout ); if self.create_next_block_on_timeout { - let empty_block_timeout_ms = - self.config.consensus.empty_block_timeout.as_millis() as u64; - // Check if enough time elapsed to propse block - if time_since_last_view_change > empty_block_timeout_ms { + if time_since_last_view_change > empty_block_timeout { if let Ok(Some((block, transactions))) = self.propose_new_block() { self.create_next_block_on_timeout = false; return Ok(Some(( @@ -514,12 +511,9 @@ impl Consensus { ))); }; } else { - self.reset_timeout.send( - self.config - .consensus - .empty_block_timeout - .saturating_sub(Duration::from_millis(time_since_last_view_change)), - )?; + self.reset_timeout.send(Duration::from_millis( + empty_block_timeout.saturating_sub(time_since_last_view_change), + ))?; return Ok(None); } } @@ -587,24 +581,24 @@ impl Consensus { .duration_since(self.view_updated_at) .unwrap_or_default() .as_millis() as u64; - let exponential_backoff_timeout = self.exponential_backoff_timeout(self.get_view()?); - - let minimum_time_left_for_empty_block = self - .config - .consensus - .minimum_time_left_for_empty_block - .as_millis() as u64; + let view_difference = self.get_view().saturating_sub(self.high_qc.view); + // in view N our highQC is the one we obtained in view N-1 (or before) and its view is N-2 (or lower) + // in other words, the current view is always at least 2 views ahead of the highQC's view + // i.e. to get `consensus_timeout_ms * 2^0` we have to subtract 2 from `view_difference` + let exponential_backoff_timeout = + consensus_timeout_ms * 2u64.pow((view_difference as u32).saturating_sub(2)); + let empty_block_timeout = self.config.consensus.empty_block_timeout.as_millis() as u64; trace!( time_since_last_view_change, exponential_backoff_timeout, - minimum_time_left_for_empty_block, + empty_block_timeout ); Ok(( time_since_last_view_change, exponential_backoff_timeout, - minimum_time_left_for_empty_block, + empty_block_timeout, )) } @@ -1355,15 +1349,10 @@ impl Consensus { // Assemble new block with whatever is in the mempool while let Some(tx) = self.transaction_pool.best_transaction(&state)? { // First - check if we have time left to process txns and give enough time for block propagation - let ( - time_since_last_view_change, - exponential_backoff_timeout, - minimum_time_left_for_empty_block, - ) = self.get_consensus_timeout_params()?; + let (time_since_last_view_change, _, empty_block_timeout) = + self.get_consensus_timeout_params(); - if time_since_last_view_change + minimum_time_left_for_empty_block - >= exponential_backoff_timeout - { + if time_since_last_view_change > empty_block_timeout { debug!( time_since_last_view_change, exponential_backoff_timeout, @@ -1475,15 +1464,10 @@ impl Consensus { /// Either propose now or set timeout to allow for txs to come in. fn ready_for_block_proposal(&mut self) -> Result)>> { // Check if there's enough time to wait on a timeout and then propagate an empty block in the network before other participants trigger NewView - let ( - time_since_last_view_change, - exponential_backoff_timeout, - minimum_time_left_for_empty_block, - ) = self.get_consensus_timeout_params()?; + let (time_since_last_view_change, _, empty_block_timeout) = + self.get_consensus_timeout_params()?; - if time_since_last_view_change + minimum_time_left_for_empty_block - >= exponential_backoff_timeout - { + if time_since_last_view_change > empty_block_timeout { return self.propose_new_block(); } @@ -1553,15 +1537,10 @@ impl Consensus { for txn in pending.into_iter() { // First - check for time - let ( - time_since_last_view_change, - exponential_backoff_timeout, - minimum_time_left_for_empty_block, - ) = self.get_consensus_timeout_params()?; + let (time_since_last_view_change, _, empty_block_timeout) = + self.get_consensus_timeout_params()?; - if time_since_last_view_change + minimum_time_left_for_empty_block - >= exponential_backoff_timeout - { + if time_since_last_view_change > empty_block_timeout { break; } diff --git a/zilliqa/tests/it/main.rs b/zilliqa/tests/it/main.rs index 975298ee5..b1ccdede8 100644 --- a/zilliqa/tests/it/main.rs +++ b/zilliqa/tests/it/main.rs @@ -321,7 +321,6 @@ impl Network { genesis_deposits: genesis_deposits.clone(), is_main: send_to_parent.is_none(), consensus_timeout: Duration::from_secs(5), - minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(), // Give a genesis account 1 billion ZIL. genesis_accounts: Self::genesis_accounts(&genesis_key), empty_block_timeout: Duration::from_millis(25), @@ -464,7 +463,6 @@ impl Network { eth_block_gas_limit: EvmGas(84000000), gas_price: 4_761_904_800_000u128.into(), main_shard_id: None, - minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(), scilla_address: scilla_address_default(), blocks_per_epoch: self.blocks_per_epoch, epochs_per_checkpoint: 1, diff --git a/zilliqa/tests/it/persistence.rs b/zilliqa/tests/it/persistence.rs index b65879445..06cc47800 100644 --- a/zilliqa/tests/it/persistence.rs +++ b/zilliqa/tests/it/persistence.rs @@ -108,7 +108,6 @@ async fn block_and_tx_data_persistence(mut network: Network) { consensus_timeout: consensus_timeout_default(), genesis_deposits: Vec::new(), main_shard_id: None, - minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(), scilla_address: scilla_address_default(), blocks_per_epoch: 10, epochs_per_checkpoint: 1, From a1e9edd1cd3303b610f9300740a6d71525498e83 Mon Sep 17 00:00:00 2001 From: Tomos Wootton Date: Thu, 12 Dec 2024 15:50:22 +0000 Subject: [PATCH 3/3] fix: rebase corrections --- z2/src/setup.rs | 7 +++---- zilliqa/src/cfg.rs | 5 ----- zilliqa/src/consensus.rs | 26 +++++++++----------------- zilliqa/tests/it/main.rs | 8 ++++---- zilliqa/tests/it/persistence.rs | 7 +++---- 5 files changed, 19 insertions(+), 34 deletions(-) diff --git a/z2/src/setup.rs b/z2/src/setup.rs index 397809afe..12486e758 100644 --- a/z2/src/setup.rs +++ b/z2/src/setup.rs @@ -20,10 +20,9 @@ use zilliqa::{ self, allowed_timestamp_skew_default, block_request_batch_size_default, block_request_limit_default, consensus_timeout_default, empty_block_timeout_default, eth_chain_id_default, failed_request_sleep_duration_default, local_address_default, - max_blocks_in_flight_default, minimum_time_left_for_empty_block_default, - scilla_address_default, scilla_ext_libs_path_default, scilla_stdlib_dir_default, - state_rpc_limit_default, total_native_token_supply_default, Amount, ApiServer, - ConsensusConfig, GenesisDeposit, + max_blocks_in_flight_default, scilla_address_default, scilla_ext_libs_path_default, + scilla_stdlib_dir_default, state_rpc_limit_default, total_native_token_supply_default, + Amount, ApiServer, ConsensusConfig, GenesisDeposit, }, transaction::EvmGas, }; diff --git a/zilliqa/src/cfg.rs b/zilliqa/src/cfg.rs index 5c618af6a..53fb0af03 100644 --- a/zilliqa/src/cfg.rs +++ b/zilliqa/src/cfg.rs @@ -345,7 +345,6 @@ impl Default for ConsensusConfig { genesis_deposits: vec![], genesis_accounts: vec![], empty_block_timeout: empty_block_timeout_default(), - minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(), scilla_address: scilla_address_default(), scilla_stdlib_dir: scilla_stdlib_dir_default(), scilla_ext_libs_path: scilla_ext_libs_path_default(), @@ -381,10 +380,6 @@ pub fn empty_block_timeout_default() -> Duration { Duration::from_millis(1000) } -pub fn minimum_time_left_for_empty_block_default() -> Duration { - Duration::from_millis(3000) -} - pub fn scilla_address_default() -> String { String::from("http://localhost:3000") } diff --git a/zilliqa/src/consensus.rs b/zilliqa/src/consensus.rs index 2efa9f450..0b41324a6 100644 --- a/zilliqa/src/consensus.rs +++ b/zilliqa/src/consensus.rs @@ -494,14 +494,14 @@ impl Consensus { } let (time_since_last_view_change, exponential_backoff_timeout, empty_block_timeout) = - self.get_consensus_timeout_params(); + self.get_consensus_timeout_params()?; trace!( "timeout reached create_next_block_on_timeout: {}", self.create_next_block_on_timeout ); if self.create_next_block_on_timeout { - // Check if enough time elapsed to propse block + // Check if enough time elapsed to propose block if time_since_last_view_change > empty_block_timeout { if let Ok(Some((block, transactions))) = self.propose_new_block() { self.create_next_block_on_timeout = false; @@ -577,16 +577,12 @@ impl Consensus { } fn get_consensus_timeout_params(&self) -> Result<(u64, u64, u64)> { + let consensus_timeout_ms = self.config.consensus.consensus_timeout.as_millis() as u64; let time_since_last_view_change = SystemTime::now() .duration_since(self.view_updated_at) .unwrap_or_default() .as_millis() as u64; - let view_difference = self.get_view().saturating_sub(self.high_qc.view); - // in view N our highQC is the one we obtained in view N-1 (or before) and its view is N-2 (or lower) - // in other words, the current view is always at least 2 views ahead of the highQC's view - // i.e. to get `consensus_timeout_ms * 2^0` we have to subtract 2 from `view_difference` - let exponential_backoff_timeout = - consensus_timeout_ms * 2u64.pow((view_difference as u32).saturating_sub(2)); + let exponential_backoff_timeout = self.exponential_backoff_timeout(self.get_view()?); let empty_block_timeout = self.config.consensus.empty_block_timeout.as_millis() as u64; trace!( @@ -1350,16 +1346,12 @@ impl Consensus { while let Some(tx) = self.transaction_pool.best_transaction(&state)? { // First - check if we have time left to process txns and give enough time for block propagation let (time_since_last_view_change, _, empty_block_timeout) = - self.get_consensus_timeout_params(); + self.get_consensus_timeout_params()?; if time_since_last_view_change > empty_block_timeout { debug!( time_since_last_view_change, - exponential_backoff_timeout, - minimum_time_left_for_empty_block, - "timeout proposal {} for view {}", - proposal.header.number, - proposal.header.view, + "timeout proposal {} for view {}", proposal.header.number, proposal.header.view, ); // don't have time break; @@ -2535,9 +2527,9 @@ impl Consensus { /// Calculate how long we should wait before timing out for this view pub fn exponential_backoff_timeout(&self, view: u64) -> u64 { let view_difference = view.saturating_sub(self.high_qc.view); - // in view N our highQC is the one we obtained in view N-1 (or before) and its view is N-2 (or lower) - // in other words, the current view is always at least 2 views ahead of the highQC's view - // i.e. to get `consensus_timeout_ms * 2^0` we have to subtract 2 from `view_difference` + // In view N our highQC is the one we obtained in view N-1 (or before) and its view is N-2 (or lower) + // in other words, the current view is always at least 2 views ahead of the highQC's view. + // Therefore to get `consensus_timeout_ms * 2^0` we have to subtract 2 from `view_difference` let consensus_timeout = self.config.consensus.consensus_timeout.as_millis() as u64; consensus_timeout * 2u64.pow((view_difference as u32).saturating_sub(2)) } diff --git a/zilliqa/tests/it/main.rs b/zilliqa/tests/it/main.rs index b1ccdede8..fd534944d 100644 --- a/zilliqa/tests/it/main.rs +++ b/zilliqa/tests/it/main.rs @@ -67,10 +67,10 @@ use zilliqa::{ cfg::{ allowed_timestamp_skew_default, block_request_batch_size_default, block_request_limit_default, eth_chain_id_default, failed_request_sleep_duration_default, - max_blocks_in_flight_default, minimum_time_left_for_empty_block_default, - scilla_address_default, scilla_ext_libs_path_default, scilla_stdlib_dir_default, - state_cache_size_default, state_rpc_limit_default, total_native_token_supply_default, - Amount, ApiServer, Checkpoint, ConsensusConfig, GenesisDeposit, NodeConfig, + max_blocks_in_flight_default, scilla_address_default, scilla_ext_libs_path_default, + scilla_stdlib_dir_default, state_cache_size_default, state_rpc_limit_default, + total_native_token_supply_default, Amount, ApiServer, Checkpoint, ConsensusConfig, + GenesisDeposit, NodeConfig, }, crypto::{SecretKey, TransactionPublicKey}, db, diff --git a/zilliqa/tests/it/persistence.rs b/zilliqa/tests/it/persistence.rs index 06cc47800..24b250893 100644 --- a/zilliqa/tests/it/persistence.rs +++ b/zilliqa/tests/it/persistence.rs @@ -13,10 +13,9 @@ use zilliqa::{ allowed_timestamp_skew_default, block_request_batch_size_default, block_request_limit_default, consensus_timeout_default, eth_chain_id_default, failed_request_sleep_duration_default, max_blocks_in_flight_default, - minimum_time_left_for_empty_block_default, scilla_address_default, - scilla_ext_libs_path_default, scilla_stdlib_dir_default, state_cache_size_default, - state_rpc_limit_default, total_native_token_supply_default, ApiServer, Checkpoint, - ConsensusConfig, NodeConfig, + scilla_address_default, scilla_ext_libs_path_default, scilla_stdlib_dir_default, + state_cache_size_default, state_rpc_limit_default, total_native_token_supply_default, + ApiServer, Checkpoint, ConsensusConfig, NodeConfig, }, crypto::{Hash, SecretKey}, transaction::EvmGas,