chore: Remove initial height and validator set from consensus params (#1190)

Closes: #XXX

These parameters are a remnant from a very early time in Malachite's
lifespan where consensus would always start at the initial height with a
static validator set and move from height to height on its own.

These parameters have therefore not had any effect for a while, given
that it is now up to the app to tell consensus which height to start at
after receiving the `ConsensusReady` message.

As such, they can be removed without any loss of functionality.

---

### PR author checklist

#### For all contributors

- [ ] Reference a GitHub issue
- [ ] Ensure the PR title follows the [conventional
commits][conv-commits] spec
- [ ] Add a release note in [`RELEASE_NOTES.md`](/RELEASE_NOTES.md) if
the change warrants it
- [ ] Add an entry in [`BREAKING_CHANGES.md`](/BREAKING_CHANGES.md) if
the change warrants it

#### For external contributors

- [ ] Maintainers of Malachite are [allowed to push changes to the
branch][gh-edit-branch]

[conv-commits]: https://www.conventionalcommits.org/en/v1.0.0/#summary
[gh-edit-branch]:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests

Author: romac

BREAKING_CHANGES.md

@@ -5,6 +5,7 @@
 ### `malachitebft-core-types`
 
 - Move `SigningProvider` and `SigningProviderExt` traits into new `malachitebft-signing` crate ([#1191](https://github.com/informalsystems/malachite/pull/1191))
+- Remove `initial_validator_set` and `initial_height` fields from `Params` struct ([#1190](https://github.com/circlefin/malachite/pull/1190))
 
 ### `malachitebft-signing`
 

RELEASE_NOTES.md

@@ -11,6 +11,7 @@
 - Add facility for app to request a consensus state dump at any time ([#1176](https://github.com/informalsystems/malachite/pull/1176))
 - Make libp2p protocol names configurable ([#1161](https://github.com/informalsystems/malachite/issues/1161))
 - Fix mismatched height of WAL entries emitted when processing `StartHeight` input ([#1232](https://github.com/circlefin/malachite/issues/1232))
+- Remove `initial_validator_set` and `initial_height` fields from `Params` struct ([#1190](https://github.com/circlefin/malachite/pull/1190))
 
 ## 0.5.0
 

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

@@ -96,7 +96,7 @@ impl From<oneshot::error::RecvError> for ConsensusRequestError {
 /// ```
 pub enum ConsensusRequest<Ctx: Context> {
     /// Request a state dump from consensus
-    DumpState(Reply<StateDump<Ctx>>),
+    DumpState(Reply<Option<StateDump<Ctx>>>),
 }
 
 impl<Ctx: Context> ConsensusRequest<Ctx> {
@@ -105,7 +105,7 @@ impl<Ctx: Context> ConsensusRequest<Ctx> {
     /// If the request fails, `None` is returned.
     pub async fn dump_state(
         tx_request: &mpsc::Sender<ConsensusRequest<Ctx>>,
-    ) -> Result<StateDump<Ctx>, ConsensusRequestError> {
+    ) -> Result<Option<StateDump<Ctx>>, ConsensusRequestError> {
         let (tx, rx) = oneshot::channel();
 
         tx_request

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

@@ -24,8 +24,6 @@ pub async fn start_engine<Node, Ctx, WalCodec, NetCodec>(
     cfg: Node::Config,
     wal_codec: WalCodec,
     net_codec: NetCodec,
-    start_height: Option<Ctx::Height>,
-    initial_validator_set: Ctx::ValidatorSet,
 ) -> Result<(Channels<Ctx>, EngineHandle)>
 where
     Ctx: Context,
@@ -35,8 +33,6 @@ where
     NetCodec: codec::ConsensusCodec<Ctx>,
     NetCodec: codec::SyncCodec<Ctx>,
 {
-    let start_height = start_height.unwrap_or_default();
-
     let registry = SharedRegistry::global().with_moniker(cfg.moniker());
     let metrics = Metrics::register(&registry);
 
@@ -70,8 +66,6 @@ where
 
     // Spawn consensus
     let consensus = spawn_consensus_actor(
-        start_height,
-        initial_validator_set,
         address,
         ctx.clone(),
         cfg.consensus().clone(),

code/crates/app/src/spawn.rs

@@ -71,8 +71,6 @@ where
 
 #[allow(clippy::too_many_arguments)]
 pub async fn spawn_consensus_actor<Ctx>(
-    initial_height: Ctx::Height,
-    initial_validator_set: Ctx::ValidatorSet,
     address: Ctx::Address,
     ctx: Ctx,
     mut cfg: ConsensusConfig,
@@ -97,8 +95,6 @@ where
     };
 
     let consensus_params = ConsensusParams {
-        initial_height,
-        initial_validator_set,
         address,
         threshold_params: Default::default(),
         value_payload,

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

@@ -11,12 +11,6 @@ pub use malachitebft_core_driver::ThresholdParams;
 /// Consensus parameters.
 #[derive_where(Clone, Debug)]
 pub struct Params<Ctx: Context> {
-    /// The initial height
-    pub initial_height: Ctx::Height,
-
-    /// The initial validator set
-    pub initial_validator_set: Ctx::ValidatorSet,
-
     /// The address of this validator
     pub address: Ctx::Address,
 

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

@@ -41,11 +41,17 @@ impl<Ctx> State<Ctx>
 where
     Ctx: Context,
 {
-    pub fn new(ctx: Ctx, params: Params<Ctx>, queue_capacity: usize) -> Self {
+    pub fn new(
+        ctx: Ctx,
+        height: Ctx::Height,
+        validator_set: Ctx::ValidatorSet,
+        params: Params<Ctx>,
+        queue_capacity: usize,
+    ) -> Self {
         let driver = Driver::new(
             ctx.clone(),
-            params.initial_height,
-            params.initial_validator_set.clone(),
+            height,
+            validator_set,
             params.address.clone(),
             params.threshold_params,
         );

code/crates/engine/src/consensus.rs

@@ -118,7 +118,7 @@ pub enum Msg<Ctx: Context> {
     RestartHeight(Ctx::Height, Ctx::ValidatorSet),
 
     /// Request to dump the current consensus state
-    DumpState(RpcReplyPort<StateDump<Ctx>>),
+    DumpState(RpcReplyPort<Option<StateDump<Ctx>>>),
 }
 
 impl<Ctx: Context> fmt::Display for Msg<Ctx> {
@@ -235,8 +235,9 @@ pub struct State<Ctx: Context> {
     /// Timeouts configuration
     timeouts: Timeouts,
 
-    /// The state of the consensus state machine
-    consensus: ConsensusState<Ctx>,
+    /// The state of the consensus state machine,
+    /// or `None` if consensus has not been started yet.
+    consensus: Option<ConsensusState<Ctx>>,
 
     /// The set of peers we are connected to.
     connected_peers: BTreeSet<PeerId>,
@@ -254,7 +255,17 @@ where
     Ctx: Context,
 {
     pub fn height(&self) -> Ctx::Height {
-        self.consensus.height()
+        self.consensus
+            .as_ref()
+            .map(|c| c.height())
+            .unwrap_or_default()
+    }
+
+    pub fn round(&self) -> Round {
+        self.consensus
+            .as_ref()
+            .map(|c| c.round())
+            .unwrap_or(Round::Nil)
     }
 
     fn set_phase(&mut self, phase: Phase) {
@@ -315,7 +326,7 @@ where
     ) -> Result<(), ConsensusError<Ctx>> {
         malachitebft_core_consensus::process!(
             input: input,
-            state: &mut state.consensus,
+            state: state.consensus.as_mut().expect("Consensus not started"),
             metrics: &self.metrics,
             with: effect => {
                 let handler_state = HandlerState {
@@ -370,6 +381,17 @@ where
                     return Err(eyre!("Validator set for height {height} is empty").into());
                 }
 
+                // Initialize consensus state if this is the first height we start
+                if state.consensus.is_none() {
+                    state.consensus = Some(ConsensusState::new(
+                        self.ctx.clone(),
+                        height,
+                        validator_set.clone(),
+                        self.params.clone(),
+                        self.consensus_config.queue_capacity,
+                    ));
+                }
+
                 self.tx_event
                     .send(|| Event::StartedHeight(height, is_restart));
 
@@ -463,13 +485,7 @@ where
                             return Ok(());
                         }
 
-                        info!(%peer_id, "Connected to peer");
-
-                        let validator_set = state.consensus.validator_set();
-                        let connected_peers = state.connected_peers.len();
-                        let total_peers = validator_set.count().saturating_sub(1);
-
-                        debug!(connected = %connected_peers, total = %total_peers, "Connected to another peer");
+                        info!(%peer_id, total = %state.connected_peers.len(), "Connected to peer");
 
                         self.metrics.connected_peers.inc();
                     }
@@ -536,7 +552,7 @@ where
                             Event::Received(SignedConsensusMsg::Proposal(proposal.clone()))
                         });
 
-                        if state.consensus.params.value_payload.parts_only() {
+                        if self.params.value_payload.parts_only() {
                             error!(%from, "Properly configured peer should never send proposal messages in BlockPart mode");
                             return Ok(());
                         }
@@ -576,7 +592,7 @@ where
                     }
 
                     NetworkEvent::ProposalPart(from, part) => {
-                        if state.consensus.params.value_payload.proposal_only() {
+                        if self.params.value_payload.proposal_only() {
                             error!(%from, "Properly configured peer should never send proposal part messages in Proposal mode");
                             return Ok(());
                         }
@@ -634,9 +650,20 @@ where
             }
 
             Msg::DumpState(reply_to) => {
-                let dump = StateDump::new(&state.consensus);
+                let state_dump = if let Some(consensus) = &state.consensus {
+                    info!(
+                        height = %consensus.height(),
+                        round  = %consensus.round(),
+                        "Dumping consensus state"
+                    );
 
-                if let Err(e) = reply_to.send(dump) {
+                    Some(StateDump::new(consensus))
+                } else {
+                    info!("Dumping consensus state: not started");
+                    None
+                };
+
+                if let Err(e) = reply_to.send(state_dump) {
                     error!("Failed to reply with state dump: {e}");
                 }
 
@@ -690,7 +717,10 @@ where
             TimeoutKind::Prevote | TimeoutKind::Precommit | TimeoutKind::Rebroadcast
         ) {
             info!(step = ?timeout.kind, "Timeout elapsed");
-            state.consensus.print_state();
+
+            state.consensus.as_ref().inspect(|consensus| {
+                consensus.print_state();
+            });
         }
 
         // Process the timeout event
@@ -1319,11 +1349,7 @@ where
         Ok(State {
             timers: Timers::new(Box::new(myself)),
             timeouts: Timeouts::new(self.consensus_config.timeouts),
-            consensus: ConsensusState::new(
-                self.ctx.clone(),
-                self.params.clone(),
-                self.consensus_config.queue_capacity,
-            ),
+            consensus: None,
             connected_peers: BTreeSet::new(),
             phase: Phase::Unstarted,
             msg_buffer: MessageBuffer::new(MAX_BUFFER_SIZE),
@@ -1334,10 +1360,7 @@ where
         name = "consensus",
         parent = &self.span,
         skip_all,
-        fields(
-            height = %state.consensus.height(),
-            round = %state.consensus.round()
-        )
+        fields(height = %state.height(), round = %state.round())
     )]
     async fn post_start(
         &self,
@@ -1355,8 +1378,8 @@ where
         parent = &self.span,
         skip_all,
         fields(
-            height = %span_height(state.consensus.height(), &msg),
-            round = %span_round(state.consensus.round(), &msg)
+            height = %span_height(state.height(), &msg),
+            round = %span_round(state.round(), &msg)
         )
     )]
     async fn handle(
@@ -1383,8 +1406,8 @@ where
         parent = &self.span,
         skip_all,
         fields(
-            height = %state.consensus.height(),
-            round = %state.consensus.round()
+            height = %state.height(),
+            round = %state.round()
         )
     )]
     async fn post_stop(

code/crates/starknet/host/src/node.rs

@@ -20,7 +20,7 @@ use malachitebft_starknet_p2p_types::Ed25519Provider;
 
 use crate::config::{load_config, Config};
 use crate::spawn::spawn_node_actor;
-use crate::types::{Address, Height, MockContext, PrivateKey, PublicKey, Validator, ValidatorSet};
+use crate::types::{Address, MockContext, PrivateKey, PublicKey, Validator, ValidatorSet};
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct Genesis {
@@ -160,14 +160,11 @@ impl Node for StarknetNode {
         let genesis = self.load_genesis()?;
         let tx_event = TxEvent::new();
 
-        let start_height = self.start_height.map(|height| Height::new(height, 1));
-
         let (actor, handle) = spawn_node_actor(
             config.clone(),
             self.home_dir.clone(),
             genesis.validator_set,
             private_key,
-            start_height,
             tx_event.clone(),
             span.clone(),
         )

code/crates/starknet/host/src/spawn.rs

@@ -28,21 +28,18 @@ use crate::mempool::{Mempool, MempoolRef};
 use crate::mempool_load::{MempoolLoad, MempoolLoadRef, Params};
 use crate::metrics::Metrics as AppMetrics;
 use crate::types::MockContext;
-use crate::types::{Address, Height, PrivateKey, ValidatorSet};
+use crate::types::{Address, PrivateKey, ValidatorSet};
 
 pub async fn spawn_node_actor(
     cfg: Config,
     home_dir: PathBuf,
     initial_validator_set: ValidatorSet,
     private_key: PrivateKey,
-    start_height: Option<Height>,
     tx_event: TxEvent<MockContext>,
     span: tracing::Span,
 ) -> (NodeRef, JoinHandle<()>) {
     let ctx = MockContext::new();
 
-    let start_height = start_height.unwrap_or(Height::new(1, 1));
-
     let registry = SharedRegistry::global().with_moniker(cfg.moniker.as_str());
     let consensus_metrics = ConsensusMetrics::register(&registry);
     let app_metrics = AppMetrics::register(&registry);
@@ -88,8 +85,6 @@ pub async fn spawn_node_actor(
 
     // Spawn consensus
     let consensus = spawn_consensus_actor(
-        start_height,
-        initial_validator_set,
         address,
         ctx,
         cfg,
@@ -179,8 +174,6 @@ async fn spawn_sync_actor(
 
 #[allow(clippy::too_many_arguments)]
 async fn spawn_consensus_actor(
-    initial_height: Height,
-    initial_validator_set: ValidatorSet,
     address: Address,
     ctx: MockContext,
     mut cfg: Config,
@@ -194,8 +187,6 @@ async fn spawn_consensus_actor(
     span: &tracing::Span,
 ) -> ConsensusRef<MockContext> {
     let consensus_params = ConsensusParams {
-        initial_height,
-        initial_validator_set,
         address,
         threshold_params: Default::default(),
         value_payload: ValuePayload::PartsOnly,