Remove the time limit related code (#1037)

Author: ancazamfir

code/crates/config/src/lib.rs

@@ -502,11 +502,6 @@ pub struct TimeoutConfig {
     #[serde(with = "humantime_serde")]
     pub timeout_precommit_delta: Duration,
 
-    /// How long we stay in preovte or precommit steps before starting
-    /// the vote synchronization protocol.
-    #[serde(with = "humantime_serde")]
-    pub timeout_step: Duration,
-
     /// How long we wait after entering a round before starting
     /// the rebroadcast liveness protocol    
     #[serde(with = "humantime_serde")]
@@ -519,8 +514,6 @@ impl TimeoutConfig {
             TimeoutKind::Propose => self.timeout_propose,
             TimeoutKind::Prevote => self.timeout_prevote,
             TimeoutKind::Precommit => self.timeout_precommit,
-            TimeoutKind::PrevoteTimeLimit => self.timeout_step,
-            TimeoutKind::PrecommitTimeLimit => self.timeout_step,
             TimeoutKind::Rebroadcast => {
                 self.timeout_propose + self.timeout_prevote + self.timeout_precommit
             }
@@ -532,8 +525,6 @@ impl TimeoutConfig {
             TimeoutKind::Propose => Some(self.timeout_propose_delta),
             TimeoutKind::Prevote => Some(self.timeout_prevote_delta),
             TimeoutKind::Precommit => Some(self.timeout_precommit_delta),
-            TimeoutKind::PrevoteTimeLimit => None,
-            TimeoutKind::PrecommitTimeLimit => None,
             TimeoutKind::Rebroadcast => None,
         }
     }
@@ -544,7 +535,6 @@ impl Default for TimeoutConfig {
         let timeout_propose = Duration::from_secs(3);
         let timeout_prevote = Duration::from_secs(1);
         let timeout_precommit = Duration::from_secs(1);
-        let timeout_step = Duration::from_secs(2);
         let timeout_rebroadcast = timeout_propose + timeout_prevote + timeout_precommit;
 
         Self {
@@ -554,7 +544,6 @@ impl Default for TimeoutConfig {
             timeout_prevote_delta: Duration::from_millis(500),
             timeout_precommit,
             timeout_precommit_delta: Duration::from_millis(500),
-            timeout_step,
             timeout_rebroadcast,
         }
     }

code/crates/core-consensus/src/handle.rs

@@ -7,7 +7,6 @@ mod proposed_value;
 mod rebroadcast_timeout;
 mod signature;
 mod start_height;
-mod step_timeout;
 mod sync;
 mod timeout;
 mod validator_set;

code/crates/core-consensus/src/handle/driver.rs

@@ -163,22 +163,12 @@ where
         }
     }
 
-    if prev_step != new_step {
-        if state.driver.step_is_prevote() {
-            // Cancel the Propose timeout since we have moved from Propose to Prevote
-            perform!(
-                co,
-                Effect::CancelTimeout(Timeout::propose(state.driver.round()), Default::default())
-            );
-        } else if state.driver.step_is_commit() {
-            perform!(
-                co,
-                Effect::CancelTimeout(
-                    Timeout::precommit_time_limit(state.driver.round()),
-                    Default::default()
-                )
-            );
-        }
+    if prev_step != new_step && state.driver.step_is_prevote() {
+        // Cancel the Propose timeout since we have moved from Propose to Prevote
+        perform!(
+            co,
+            Effect::CancelTimeout(Timeout::propose(state.driver.round()), Default::default())
+        );
     }
 
     process_driver_outputs(co, state, metrics, outputs).await?;

code/crates/core-consensus/src/handle/step_timeout.rs

@@ -1,6 +1,5 @@
 use crate::handle::driver::apply_driver_input;
 use crate::handle::rebroadcast_timeout::on_rebroadcast_timeout;
-use crate::handle::step_timeout::on_step_limit_timeout;
 use crate::prelude::*;
 use crate::types::WalEntry;
 
@@ -49,11 +48,9 @@ where
 
     apply_driver_input(co, state, metrics, DriverInput::TimeoutElapsed(timeout)).await?;
 
-    match timeout.kind {
-        TimeoutKind::Rebroadcast => on_rebroadcast_timeout(co, state, metrics).await,
-        TimeoutKind::PrevoteTimeLimit | TimeoutKind::PrecommitTimeLimit => {
-            on_step_limit_timeout(co, state, metrics, timeout.round).await
-        }
-        _ => Ok(()),
+    if matches!(timeout.kind, TimeoutKind::Rebroadcast) {
+        on_rebroadcast_timeout(co, state, metrics).await?;
     }
+
+    Ok(())
 }

code/crates/core-consensus/src/handle/timeout.rs

@@ -548,8 +548,6 @@ where
             TimeoutKind::Precommit => RoundInput::TimeoutPrecommit,
 
             // The driver never receives these events, so we can just ignore them.
-            TimeoutKind::PrevoteTimeLimit => return Ok(None),
-            TimeoutKind::PrecommitTimeLimit => return Ok(None),
             TimeoutKind::Rebroadcast => return Ok(None),
         };
 

code/crates/core-driver/src/driver.rs

@@ -11,15 +11,9 @@ pub enum TimeoutKind {
     /// Timeout for the prevote step.
     Prevote,
 
-    /// Timeout for detecting consensus being in the prevote step for too long.
-    PrevoteTimeLimit,
-
     /// Timeout for the precommit step.
     Precommit,
 
-    /// Timeout for detecting consensus being in the precommit step for too long.
-    PrecommitTimeLimit,
-
     /// Timeout to rebroadcast the round synchronization messages
     Rebroadcast,
 }
@@ -50,19 +44,10 @@ impl Timeout {
         Self::new(round, TimeoutKind::Prevote)
     }
 
-    /// Create a new timeout for the prevote step of the given round.
-    pub const fn prevote_time_limit(round: Round) -> Self {
-        Self::new(round, TimeoutKind::PrevoteTimeLimit)
-    }
-
     /// Create a new timeout for the precommit step of the given round.
     pub const fn precommit(round: Round) -> Self {
         Self::new(round, TimeoutKind::Precommit)
     }
-    /// Create a new timeout for the precommit step of the given round.
-    pub const fn precommit_time_limit(round: Round) -> Self {
-        Self::new(round, TimeoutKind::PrecommitTimeLimit)
-    }
 
     /// Create a new timeout for rebroadcasting the round synchronization messages.
     pub const fn rebroadcast(round: Round) -> Self {

code/crates/core-types/src/timeout.rs

@@ -185,8 +185,6 @@ impl Timeouts {
             TimeoutKind::Propose => self.config.timeout_propose,
             TimeoutKind::Prevote => self.config.timeout_prevote,
             TimeoutKind::Precommit => self.config.timeout_precommit,
-            TimeoutKind::PrevoteTimeLimit => self.config.timeout_step,
-            TimeoutKind::PrecommitTimeLimit => self.config.timeout_step,
             TimeoutKind::Rebroadcast => {
                 self.config.timeout_propose
                     + self.config.timeout_prevote
@@ -201,8 +199,6 @@ impl Timeouts {
             TimeoutKind::Propose => c.timeout_propose += c.timeout_propose_delta,
             TimeoutKind::Prevote => c.timeout_prevote += c.timeout_prevote_delta,
             TimeoutKind::Precommit => c.timeout_precommit += c.timeout_precommit_delta,
-            TimeoutKind::PrevoteTimeLimit => (),
-            TimeoutKind::PrecommitTimeLimit => (),
             TimeoutKind::Rebroadcast => {
                 c.timeout_rebroadcast +=
                     c.timeout_propose_delta + c.timeout_prevote_delta + c.timeout_precommit_delta
@@ -640,10 +636,7 @@ where
         // Print debug information if the timeout is for a prevote or precommit
         if matches!(
             timeout.kind,
-            TimeoutKind::Prevote
-                | TimeoutKind::Precommit
-                | TimeoutKind::PrevoteTimeLimit
-                | TimeoutKind::PrecommitTimeLimit
+            TimeoutKind::Prevote | TimeoutKind::Precommit | TimeoutKind::Rebroadcast
         ) {
             warn!(step = ?timeout.kind, "Timeout elapsed");
             state.consensus.print_state();

code/crates/engine/src/consensus.rs

@@ -123,8 +123,6 @@ fn encode_timeout(tag: u8, timeout: &Timeout, mut buf: impl Write) -> io::Result
 
         // Consensus will typically not want to store these timeouts in the WAL,
         // but we still need to handle them here.
-        TimeoutKind::PrevoteTimeLimit => 5,
-        TimeoutKind::PrecommitTimeLimit => 6,
         TimeoutKind::Rebroadcast => 7,
     };
 
@@ -154,9 +152,7 @@ fn decode_timeout(mut buf: impl Read) -> io::Result<Timeout> {
 
         // Consensus will typically not want to store these timeouts in the WAL,
         // but we still need to handle them here.
-        5 => TimeoutKind::PrevoteTimeLimit,
-        6 => TimeoutKind::PrecommitTimeLimit,
-        7 => TimeoutKind::Rebroadcast,
+        5 => TimeoutKind::Rebroadcast,
 
         _ => {
             return Err(io::Error::new(

code/crates/engine/src/wal/entry.rs

@@ -78,9 +78,6 @@ pub struct Inner {
     /// The round of the proposal that was decided on
     pub proposal_round: Histogram,
 
-    /// Number of times consensus was blocked in Prevote or Precommit step and required vote synchronization
-    pub step_timeouts: Counter,
-
     /// Number of times consensus rebroadcasted Prevote or Precommit votes due to no round progress
     pub rebroadcast_timeouts: Counter,
 
@@ -123,7 +120,6 @@ impl Metrics {
             block_size_bytes: Histogram::new(linear_buckets(0.0, 64.0 * 1024.0, 128)),
             consensus_round: Histogram::new(linear_buckets(0.0, 1.0, 20)),
             proposal_round: Histogram::new(linear_buckets(0.0, 1.0, 20)),
-            step_timeouts: Counter::default(),
             rebroadcast_timeouts: Counter::default(),
             connected_peers: Gauge::default(),
             height: Gauge::default(),
@@ -194,16 +190,10 @@ impl Metrics {
                 metrics.proposal_round.clone(),
             );
 
-            registry.register(
-                "step_timeouts",
-                "Number of times consensus was blocked and required vote synchronization",
-                metrics.step_timeouts.clone(),
-            );
-
             registry.register(
                 "rebroadcast_timeouts",
                 "Number of times consensus rebroadcasted its vote due to no round progress",
-                metrics.step_timeouts.clone(),
+                metrics.rebroadcast_timeouts.clone(),
             );
 
             registry.register(

code/crates/metrics/src/metrics.rs

@@ -133,13 +133,11 @@ impl TransportProtocol {
 
 /// sync event details:
 ///
-/// peer1: sync               peer2: network       peer2: sync           peer1: network
-///                                                                or consensus
+/// peer1: sync                  peer2: network                    peer2: sync              peer1: network
 /// CtrlMsg::SyncRequest       --> Event::Sync      -----------> CtrlMsg::SyncReply ------> Event::Sync
 /// (peer_id, height)             (RawMessage::Request           (request_id, height)       RawMessage::Response
 ///                           {request_id, peer_id, request}                                {request_id, response}
 ///
-/// - request can be for a block or vote set
 ///
 /// An event that can be emitted by the gossip layer
 #[derive(Clone, Debug)]

code/crates/network/src/lib.rs

@@ -230,7 +230,6 @@ fn apply_params(config: &mut Config, params: &TestParams) {
     config.consensus.value_payload = ValuePayload::PartsOnly;
     config.value_sync.enabled = params.enable_value_sync;
     config.consensus.p2p.protocol = params.protocol;
-    config.consensus.timeouts.timeout_step = params.timeout_step;
     config.test.max_block_size = params.block_size;
     config.test.txs_per_part = params.txs_per_part;
     config.test.vote_extensions.enabled = params.vote_extensions.is_some();

code/crates/starknet/test/src/lib.rs

@@ -71,7 +71,7 @@ pub enum Input<Ctx: Context> {
     /// Got a response from the application to our `GetValue` request
     GotDecidedValue(InboundRequestId, Ctx::Height, Option<RawDecidedValue<Ctx>>),
 
-    /// A request for a value or vote set timed out
+    /// A request for a value timed out
     SyncRequestTimedOut(PeerId, Request<Ctx>),
 
     /// We received an invalid [`CommitCertificate`]

code/crates/sync/src/handle.rs

@@ -57,10 +57,6 @@ timeout_precommit_delta = "500ms"
 # Override with MALACHITE__CONSENSUS__TIMEOUT_COMMIT env variable
 timeout_commit = "0s"
 
-# How long we wait in the prevote and precommit steps before starting
-# the vote synchronization protocol
-# Override with MALACHITE__CONSENSUS__TIMEOUT_STEP env variable
-timeout_step = "2s"
 
 # How long we wait after entering a round before starting the rebroadcast liveness protocol
 # Override with MALACHITE__CONSENSUS__TIMEOUT_REBROADCAST env variable

code/crates/test/app/config.toml

@@ -40,7 +40,6 @@ impl TestParams {
     pub fn apply_to_config(&self, config: &mut Config) {
         config.value_sync.enabled = self.enable_value_sync;
         config.consensus.p2p.protocol = self.protocol;
-        config.consensus.timeouts.timeout_step = self.timeout_step;
         config.consensus.value_payload = self.value_payload;
         config.test.max_block_size = self.block_size;
         config.test.txs_per_part = self.txs_per_part;

code/crates/test/framework/src/params.rs

@@ -144,10 +144,7 @@ impl TestRunner {
             consensus: ConsensusConfig {
                 // Current test app does not support proposal-only value payload properly as Init does not include valid_round
                 value_payload: ValuePayload::ProposalAndParts,
-                timeouts: TimeoutConfig {
-                    timeout_step: Duration::from_secs(2),
-                    ..Default::default()
-                },
+                timeouts: TimeoutConfig::default(),
                 p2p: P2pConfig {
                     protocol,
                     discovery: DiscoveryConfig::default(),

code/crates/test/tests/it/main.rs

@@ -57,11 +57,6 @@ timeout_precommit_delta = "500ms"
 # Override with MALACHITE__CONSENSUS__TIMEOUT_COMMIT env variable
 timeout_commit = "0s"
 
-# How long we wait in the prevote and precommit steps before starting
-# the vote synchronization protocol
-# Override with MALACHITE__CONSENSUS__TIMEOUT_STEP env variable
-timeout_step = "2s"
-
 # How long we wait after entering a round before starting the rebroadcast liveness protocol
 # Override with MALACHITE__CONSENSUS__TIMEOUT_REBROADCAST env variable
 timeout_rebroadcast = "5s"