chore(code)!: Remove VoteSet-based synchronization protocol (#1008)

Author: romac

BREAKING_CHANGES.md

@@ -2,11 +2,19 @@
 
 ## Unreleased
 
+### `malachitebft-core-types`
+- Removed the VoteSet synchronization protocol, as it is neither required nor sufficient for liveness.
+  See ([#998](https://github.com/informalsystems/malachite/issues/998)) for more details.
+
 ### `malachitebft-core-consensus`
+- Removed the VoteSet synchronization protocol, as it is neither required nor sufficient for liveness.
+  See ([#998](https://github.com/informalsystems/malachite/issues/998)) for more details.
 - Added new variants to `Input` enum: `PolkaCertificate` and `RoundCertificate`
 - Added new variant to `Effect` enum: `PublishLivenessMessage`
 
 ### `malachitebft-engine`
+- Removed the VoteSet synchronization protocol, as it is neither required nor sufficient for liveness.
+  See ([#998](https://github.com/informalsystems/malachite/issues/998)) for more details.
 - Changed the reply channel of `GetValidatorSet` message to take an `Option<Ctx::ValidatorSet>` instead of `Ctx::ValidatorSet`.
 - Added new variant to `Msg` enum: `PublishLivenessMsg`
 - Added new variants to `NetworkEvent` enum: `PolkaCertificate` and `RoundCertificate`
@@ -23,6 +31,10 @@
 - Renamed `Event::Message` variant to `Event::ConsensusMessage`
 - Added new variant to `Event::LivenessMessage`
 
+### `malachitebft-sync`
+- Removed the VoteSet synchronization protocol, as it is neither required nor sufficient for liveness.
+  See ([#998](https://github.com/informalsystems/malachite/issues/998)) for more details.
+
 ## 0.2.0
 
 ### `malachitebft-core-types`

RELEASE_NOTES.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Removed the VoteSet synchronization protocol, as it is neither required nor sufficient for liveness.
+  See ([#998](https://github.com/informalsystems/malachite/issues/998)) for more details.
 - Reply to `GetValidatorSet` is now optional ([#990](https://github.com/informalsystems/malachite/issues/990))
 - Clarify and improve the application handling of multiple proposals for same height and round ([#833](https://github.com/informalsystems/malachite/issues/833))
 

code/crates/app-channel/src/run.rs

@@ -56,7 +56,6 @@ where
         network.clone(),
         connector.clone(),
         cfg.value_sync(),
-        &cfg.consensus().vote_sync,
         &registry,
     )
     .await?;

code/crates/app/src/spawn.rs

@@ -16,11 +16,11 @@ use malachitebft_engine::util::events::TxEvent;
 use malachitebft_engine::wal::{Wal, WalCodec, WalRef};
 use malachitebft_network::{Config as NetworkConfig, DiscoveryConfig, GossipSubConfig, Keypair};
 
-use crate::config::{self, ConsensusConfig, PubSubProtocol, ValueSyncConfig, VoteSyncConfig};
+use crate::config::{ConsensusConfig, PubSubProtocol, ValueSyncConfig};
 use crate::metrics::{Metrics, SharedRegistry};
 use crate::types::core::{Context, SigningProvider};
 use crate::types::sync;
-use crate::types::{ValuePayload, VoteSyncMode};
+use crate::types::ValuePayload;
 
 pub async fn spawn_node_actor<Ctx>(
     ctx: Ctx,
@@ -92,18 +92,12 @@ where
         config::ValuePayload::ProposalAndParts => ValuePayload::ProposalAndParts,
     };
 
-    let vote_sync_mode = match cfg.vote_sync.mode {
-        config::VoteSyncMode::RequestResponse => VoteSyncMode::RequestResponse,
-        config::VoteSyncMode::Rebroadcast => VoteSyncMode::Rebroadcast,
-    };
-
     let consensus_params = ConsensusParams {
         initial_height,
         initial_validator_set,
         address,
         threshold_params: Default::default(),
         value_payload,
-        vote_sync_mode,
     };
 
     Consensus::spawn(
@@ -148,13 +142,12 @@ pub async fn spawn_sync_actor<Ctx>(
     network: NetworkRef<Ctx>,
     host: HostRef<Ctx>,
     config: &ValueSyncConfig,
-    vote_sync: &VoteSyncConfig,
     registry: &SharedRegistry,
 ) -> Result<Option<SyncRef<Ctx>>>
 where
     Ctx: Context,
 {
-    if !config.enabled && vote_sync.mode != config::VoteSyncMode::RequestResponse {
+    if !config.enabled {
         return Ok(None);
     }
 

code/crates/app/src/types.rs

@@ -3,7 +3,7 @@
 pub use libp2p_identity::Keypair;
 
 pub use malachitebft_core_consensus::{
-    ConsensusMsg, ProposedValue, SignedConsensusMsg, ValuePayload, VoteSyncMode,
+    ConsensusMsg, ProposedValue, SignedConsensusMsg, ValuePayload,
 };
 pub use malachitebft_engine::host::LocallyProposedValue;
 pub use malachitebft_peer::PeerId;

code/crates/config/src/lib.rs

@@ -447,39 +447,6 @@ pub struct ConsensusConfig {
 
     /// Message types that can carry values
     pub value_payload: ValuePayload,
-
-    /// VoteSync configuration options
-    pub vote_sync: VoteSyncConfig,
-}
-
-#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
-pub struct VoteSyncConfig {
-    /// The mode of vote synchronization
-    /// - RequestResponse: The lagging node sends a request to a peer for the missing votes
-    /// - Rebroadcast: Nodes rebroadcast their last vote to all peers
-    pub mode: VoteSyncMode,
-}
-
-/// The mode of vote synchronization
-#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-pub enum VoteSyncMode {
-    /// The lagging node sends a request to a peer for the missing votes
-    #[default]
-    RequestResponse,
-
-    /// Nodes rebroadcast their last vote to all peers
-    Rebroadcast,
-}
-
-impl VoteSyncMode {
-    pub fn is_request_response(&self) -> bool {
-        matches!(self, Self::RequestResponse)
-    }
-
-    pub fn is_rebroadcast(&self) -> bool {
-        matches!(self, Self::Rebroadcast)
-    }
 }
 
 /// Message types required by consensus to deliver the value being proposed
@@ -535,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")]
@@ -552,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
             }
@@ -565,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,
         }
     }
@@ -577,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 {
@@ -587,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/effect.rs

@@ -2,7 +2,6 @@ use derive_where::derive_where;
 
 use malachitebft_core_types::*;
 
-use crate::input::RequestId;
 use crate::types::{LivenessMsg, SignedConsensusMsg};
 use crate::{ConsensusMsg, VoteExtensionError, WalEntry};
 
@@ -185,23 +184,6 @@ where
         resume::CertificateValidity,
     ),
 
-    /// Consensus has been stuck in Prevote or Precommit step, ask for vote sets from peers
-    ///
-    /// Resume with: [`resume::Continue`]
-    RequestVoteSet(Ctx::Height, Round, resume::Continue),
-
-    /// A peer has required our vote set, send the response
-    ///
-    /// Resume with: [`resume::Continue`]`
-    SendVoteSetResponse(
-        RequestId,
-        Ctx::Height,
-        Round,
-        VoteSet<Ctx>,
-        Vec<PolkaCertificate<Ctx>>,
-        resume::Continue,
-    ),
-
     /// Append an entry to the Write-Ahead Log for crash recovery
     ///
     /// Resume with: [`resume::Continue`]`

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

@@ -7,12 +7,10 @@ mod proposed_value;
 mod rebroadcast_timeout;
 mod signature;
 mod start_height;
-mod step_timeout;
 mod sync;
 mod timeout;
 mod validator_set;
 mod vote;
-mod vote_set;
 
 use liveness::{on_polka_certificate, on_round_certificate};
 use proposal::on_proposal;
@@ -22,7 +20,6 @@ use start_height::reset_and_start_height;
 use sync::on_commit_certificate;
 use timeout::on_timeout_elapsed;
 use vote::on_vote;
-use vote_set::{on_vote_set_request, on_vote_set_response};
 
 use crate::prelude::*;
 
@@ -63,12 +60,6 @@ where
         Input::CommitCertificate(certificate) => {
             on_commit_certificate(co, state, metrics, certificate).await
         }
-        Input::VoteSetRequest(request_id, height, round) => {
-            on_vote_set_request(co, state, metrics, request_id, height, round).await
-        }
-        Input::VoteSetResponse(vote_set, polka_certificate) => {
-            on_vote_set_response(co, state, metrics, vote_set, polka_certificate).await
-        }
         Input::PolkaCertificate(certificate) => {
             on_polka_certificate(co, state, metrics, certificate).await
         }

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

@@ -8,10 +8,10 @@ use crate::handle::signature::sign_vote;
 use crate::handle::vote::on_vote;
 use crate::params::HIDDEN_LOCK_ROUND;
 use crate::prelude::*;
-use crate::types::{LivenessMsg, SignedConsensusMsg};
+use crate::types::{
+    LivenessMsg, {LocallyProposedValue, SignedConsensusMsg},
+};
 use crate::util::pretty::PrettyVal;
-use crate::LocallyProposedValue;
-use crate::VoteSyncMode;
 
 use super::propose::on_propose;
 
@@ -69,11 +69,9 @@ where
             #[cfg(feature = "metrics")]
             metrics.rebroadcast_timeouts.inc();
 
-            // Schedule rebroadcast timer if necessary
-            if state.params.vote_sync_mode == VoteSyncMode::Rebroadcast {
-                let timeout = Timeout::rebroadcast(*round);
-                perform!(co, Effect::ScheduleTimeout(timeout, Default::default()));
-            }
+            // Schedule rebroadcast timer
+            let timeout = Timeout::rebroadcast(*round);
+            perform!(co, Effect::ScheduleTimeout(timeout, Default::default()));
         }
 
         DriverInput::ProposeValue(round, _) => {
@@ -165,53 +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())
-            );
-            if state.params.vote_sync_mode == VoteSyncMode::RequestResponse {
-                // Schedule the Prevote time limit timeout
-                perform!(
-                    co,
-                    Effect::ScheduleTimeout(
-                        Timeout::prevote_time_limit(state.driver.round()),
-                        Default::default()
-                    )
-                );
-            }
-        }
-
-        if state.driver.step_is_precommit()
-            && state.params.vote_sync_mode == VoteSyncMode::RequestResponse
-        {
-            perform!(
-                co,
-                Effect::CancelTimeout(
-                    Timeout::prevote_time_limit(state.driver.round()),
-                    Default::default()
-                )
-            );
-            perform!(
-                co,
-                Effect::ScheduleTimeout(
-                    Timeout::precommit_time_limit(state.driver.round()),
-                    Default::default()
-                )
-            );
-        }
-
-        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?;
@@ -405,6 +362,10 @@ where
                 );
 
                 state.set_last_vote(signed_vote);
+
+                // Schedule rebroadcast timer
+                let timeout = Timeout::rebroadcast(state.driver.round());
+                perform!(co, Effect::ScheduleTimeout(timeout, Default::default()));
             }
 
             Ok(())

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

@@ -1,4 +1,4 @@
-use crate::{prelude::*, VoteSyncMode};
+use crate::prelude::*;
 
 #[cfg_attr(not(feature = "metrics"), allow(unused_variables))]
 pub async fn on_rebroadcast_timeout<Ctx>(
@@ -9,10 +9,6 @@ pub async fn on_rebroadcast_timeout<Ctx>(
 where
     Ctx: Context,
 {
-    if state.params.vote_sync_mode != VoteSyncMode::Rebroadcast {
-        return Ok(());
-    }
-
     let (height, round) = (state.driver.height(), state.driver.round());
 
     if let Some(vote) = state.last_signed_prevote.as_ref() {

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

@@ -62,6 +62,8 @@ where
     Ok(result)
 }
 
+// NOTE: Will be used again in #997
+#[allow(dead_code)]
 pub async fn verify_polka_certificate<Ctx>(
     co: &Co<Ctx>,
     certificate: PolkaCertificate<Ctx>,