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

BREAKING_CHANGES.md

@@ -2,7 +2,17 @@
 
 ## 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.
+
 ### `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`.
 - Changed `PartStore::all_parts` to `PartStore::all_parts_by_stream_id`:
   - Renamed method to clarify that, when a new part is received, the contiguous parts should be queried by stream id
@@ -11,6 +21,10 @@
 - Added `&StreamId` parameter to `part_store::PartStore::store`
 - Added `&StreamId` parameter to `part_store::PartStore::store_value_id`
 
+### `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

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::SignedConsensusMsg;
 use crate::{ConsensusMsg, VoteExtensionError, WalEntry};
 
@@ -175,23 +174,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

@@ -11,7 +11,6 @@ mod sync;
 mod timeout;
 mod validator_set;
 mod vote;
-mod vote_set;
 
 use proposal::on_proposal;
 use propose::on_propose;
@@ -20,7 +19,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::*;
 
@@ -61,11 +59,5 @@ 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
-        }
     }
 }

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

@@ -7,10 +7,8 @@ use crate::handle::signature::sign_proposal;
 use crate::handle::signature::sign_vote;
 use crate::handle::vote::on_vote;
 use crate::prelude::*;
-use crate::types::SignedConsensusMsg;
+use crate::types::{LocallyProposedValue, SignedConsensusMsg};
 use crate::util::pretty::PrettyVal;
-use crate::LocallyProposedValue;
-use crate::VoteSyncMode;
 
 use super::propose::on_propose;
 
@@ -134,38 +132,7 @@ where
                 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() {
+        } else if state.driver.step_is_commit() {
             perform!(
                 co,
                 Effect::CancelTimeout(
@@ -287,15 +254,13 @@ where
 
                 state.set_last_vote(signed_vote);
 
-                // Schedule rebroadcast timer if necessary
-                if state.params.vote_sync_mode == VoteSyncMode::Rebroadcast {
-                    let timeout = match vote_type {
-                        VoteType::Prevote => Timeout::prevote_rebroadcast(state.driver.round()),
-                        VoteType::Precommit => Timeout::precommit_rebroadcast(state.driver.round()),
-                    };
+                // Schedule rebroadcast timer
+                let timeout = match vote_type {
+                    VoteType::Prevote => Timeout::prevote_rebroadcast(state.driver.round()),
+                    VoteType::Precommit => Timeout::precommit_rebroadcast(state.driver.round()),
+                };
 
-                    perform!(co, Effect::ScheduleTimeout(timeout, Default::default()));
-                }
+                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>(
@@ -10,10 +10,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());
 
     let (maybe_vote, timeout) = match timeout.kind {

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>,

code/crates/core-consensus/src/handle/step_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_step_limit_timeout<Ctx>(
@@ -15,15 +15,6 @@ where
         "Consensus is halted in {:?} step", state.driver.step()
     );
 
-    if state.params.vote_sync_mode == VoteSyncMode::RequestResponse {
-        warn!(height = %state.driver.height(), %round, "Requesting vote set");
-
-        perform!(
-            co,
-            Effect::RequestVoteSet(state.driver.height(), round, Default::default())
-        );
-    }
-
     #[cfg(feature = "metrics")]
     metrics.step_timeouts.inc();