chore(code/staknet): Enforce parts-only configuration and mode for st…

…arknet app (#848)

* Enforce parts-only configuration and mode for starknet app

* Revert spawn.bash change

* Small cleanup

* Post-merge fixes

* Remove vote extensions from starknet app

---------

Co-authored-by: Romain Ruetschi <romain@informal.systems>

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

@@ -11,9 +11,7 @@ use tokio::time::Instant;
 use tracing::{debug, error, info, trace, warn};
 
 use malachitebft_core_consensus::{PeerId, VoteExtensionError};
-use malachitebft_core_types::{
-    CommitCertificate, Round, Validity, ValueId, ValueOrigin, VoteExtensions,
-};
+use malachitebft_core_types::{CommitCertificate, Round, Validity, ValueId, ValueOrigin};
 use malachitebft_engine::consensus::{ConsensusMsg, ConsensusRef};
 use malachitebft_engine::host::{LocallyProposedValue, ProposedValue};
 use malachitebft_engine::network::{NetworkMsg, NetworkRef};
@@ -186,19 +184,9 @@ impl Host {
 
             HostMsg::Decided {
                 certificate,
-                extensions,
                 consensus,
-            } => {
-                on_decided(
-                    state,
-                    &consensus,
-                    &self.mempool,
-                    certificate,
-                    extensions,
-                    &self.metrics,
-                )
-                .await
-            }
+                ..
+            } => on_decided(state, &consensus, &self.mempool, certificate, &self.metrics).await,
 
             HostMsg::GetDecidedValue { height, reply_to } => {
                 on_get_decided_block(height, state, reply_to).await
@@ -341,24 +329,20 @@ async fn on_get_value(
     while let Some(part) = rx_part.recv().await {
         state.host.part_store.store(height, round, part.clone());
 
-        if state.host.params.value_payload.include_parts() {
-            debug!(%stream_id, %sequence, "Broadcasting proposal part");
+        debug!(%stream_id, %sequence, "Broadcasting proposal part");
 
-            let msg = StreamMessage::new(
-                stream_id.clone(),
-                sequence,
-                StreamContent::Data(part.clone()),
-            );
-            network.cast(NetworkMsg::PublishProposalPart(msg))?;
-        }
+        let msg = StreamMessage::new(
+            stream_id.clone(),
+            sequence,
+            StreamContent::Data(part.clone()),
+        );
+        network.cast(NetworkMsg::PublishProposalPart(msg))?;
 
         sequence += 1;
     }
 
-    if state.host.params.value_payload.include_parts() {
-        let msg = StreamMessage::new(stream_id, sequence, StreamContent::Fin);
-        network.cast(NetworkMsg::PublishProposalPart(msg))?;
-    }
+    let msg = StreamMessage::new(stream_id, sequence, StreamContent::Fin);
+    network.cast(NetworkMsg::PublishProposalPart(msg))?;
 
     let block_hash = rx_hash.await?;
     debug!(%block_hash, "Assembled block");
@@ -478,16 +462,13 @@ async fn on_restream_proposal(
 
         state.host.part_store.store(height, round, new_part.clone());
 
-        if state.host.params.value_payload.include_parts() {
-            debug!(%stream_id, %sequence, "Broadcasting proposal part");
+        debug!(%stream_id, %sequence, "Broadcasting proposal part");
 
-            let msg =
-                StreamMessage::new(stream_id.clone(), sequence, StreamContent::Data(new_part));
+        let msg = StreamMessage::new(stream_id.clone(), sequence, StreamContent::Data(new_part));
 
-            network.cast(NetworkMsg::PublishProposalPart(msg))?;
+        network.cast(NetworkMsg::PublishProposalPart(msg))?;
 
-            sequence += 1;
-        }
+        sequence += 1;
     }
 
     Ok(())
@@ -633,7 +614,6 @@ async fn on_decided(
     consensus: &ConsensusRef<MockContext>,
     mempool: &MempoolRef,
     certificate: CommitCertificate<MockContext>,
-    extensions: VoteExtensions<MockContext>,
     metrics: &Metrics,
 ) -> Result<(), ActorProcessingErr> {
     let (height, round) = (certificate.height, certificate.round);
@@ -659,9 +639,7 @@ async fn on_decided(
 
     // Update metrics
     let tx_count: usize = all_parts.iter().map(|p| p.tx_count()).sum();
-    let parts_size: usize = all_parts.iter().map(|p| p.size_bytes()).sum();
-    let extensions_size = extensions.size_bytes();
-    let block_size = parts_size + extensions_size;
+    let block_size: usize = all_parts.iter().map(|p| p.size_bytes()).sum();
 
     metrics.block_tx_count.observe(tx_count as f64);
     metrics.block_size_bytes.observe(block_size as f64);
@@ -686,7 +664,6 @@ async fn on_decided(
     mempool.cast(MempoolMsg::Update { tx_hashes })?;
 
     // Notify Starknet Host of the decision
-    // TODO: Pass extensions along as well?
     state.host.decision(certificate).await;
 
     // Start the next height

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

@@ -22,7 +22,6 @@ pub async fn build_proposal_task(
     round: Round,
     proposer: Address,
     private_key: PrivateKey,
-    // vote_extensions: VoteExtensions<MockContext>,
     params: StarknetParams,
     deadline: Instant,
     mempool: MempoolRef,
@@ -34,7 +33,6 @@ pub async fn build_proposal_task(
         round,
         proposer,
         private_key,
-        // vote_extensions,
         params,
         deadline,
         mempool,
@@ -52,7 +50,6 @@ async fn run_build_proposal_task(
     round: Round,
     proposer: Address,
     _private_key: PrivateKey,
-    // vote_extensions: VoteExtensions<MockContext>,
     params: StarknetParams,
     deadline: Instant,
     mempool: MempoolRef,

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

@@ -7,8 +7,6 @@ use tokio::sync::{mpsc, oneshot};
 use tokio::time::Instant;
 use tracing::Instrument;
 
-// use malachitebft_config::VoteExtensionsConfig;
-use malachitebft_core_consensus::ValuePayload;
 use malachitebft_core_types::{CommitCertificate, Round, SignedVote};
 
 use crate::host::Host;
@@ -21,13 +19,11 @@ use super::proposal::{build_proposal_task, repropose_task};
 #[derive(Copy, Clone, Debug)]
 pub struct StarknetParams {
     pub max_block_size: ByteSize,
-    pub value_payload: ValuePayload,
     pub tx_size: ByteSize,
     pub txs_per_part: usize,
     pub time_allowance_factor: f32,
     pub exec_time_per_tx: Duration,
     pub max_retain_blocks: usize,
-    // pub vote_extensions: VoteExtensionsConfig,
 }
 
 pub struct StarknetHost {
@@ -37,7 +33,6 @@ pub struct StarknetHost {
     pub private_key: PrivateKey,
     pub validator_set: ValidatorSet,
     pub part_store: PartStore<MockContext>,
-    // pub vote_extensions: HashMap<Height, VoteExtensions<MockContext>>,
 }
 
 impl StarknetHost {
@@ -55,25 +50,8 @@ impl StarknetHost {
             private_key,
             validator_set,
             part_store: Default::default(),
-            // vote_extensions: Default::default(),
         }
     }
-
-    // pub fn generate_vote_extension(&self, _height: Height, _round: Round) -> Option<Bytes> {
-    //     use rand::RngCore;
-    //
-    //     if !self.params.vote_extensions.enabled {
-    //         return None;
-    //     }
-    //
-    //     let size = self.params.vote_extensions.size.as_u64() as usize;
-    //     debug!(%size, "Vote extensions are enabled");
-    //
-    //     let mut bytes = vec![0u8; size];
-    //     rand::thread_rng().fill_bytes(&mut bytes);
-    //
-    //     Some(Bytes::from(bytes))
-    // }
 }
 
 #[async_trait]
@@ -100,8 +78,6 @@ impl Host for StarknetHost {
         let (tx_part, rx_content) = mpsc::channel(self.params.txs_per_part);
         let (tx_block_hash, rx_block_hash) = oneshot::channel();
 
-        // let vote_extensions = self.vote_extensions.remove(&height).unwrap_or_default();
-
         tokio::spawn(
             build_proposal_task(
                 height,

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

@@ -2,23 +2,23 @@ use std::path::{Path, PathBuf};
 use std::time::Duration;
 
 use libp2p_identity::ecdsa;
-use malachitebft_engine::util::events::TxEvent;
-use malachitebft_engine::wal::{Wal, WalRef};
-use malachitebft_starknet_p2p_types::EcdsaProvider;
 use tokio::task::JoinHandle;
+use tracing::warn;
 
 use malachitebft_config::{
     self as config, Config as NodeConfig, MempoolConfig, SyncConfig, TestConfig, TransportProtocol,
 };
-use malachitebft_core_consensus::ValuePayload;
+use malachitebft_core_types::ValuePayload;
 use malachitebft_engine::consensus::{Consensus, ConsensusParams, ConsensusRef};
 use malachitebft_engine::host::HostRef;
 use malachitebft_engine::network::{Network, NetworkRef};
 use malachitebft_engine::node::{Node, NodeRef};
 use malachitebft_engine::sync::{Params as SyncParams, Sync, SyncRef};
-use malachitebft_metrics::Metrics;
-use malachitebft_metrics::SharedRegistry;
+use malachitebft_engine::util::events::TxEvent;
+use malachitebft_engine::wal::{Wal, WalRef};
+use malachitebft_metrics::{Metrics, SharedRegistry};
 use malachitebft_network::Keypair;
+use malachitebft_starknet_p2p_types::EcdsaProvider;
 use malachitebft_sync as sync;
 use malachitebft_test_mempool::Config as MempoolNetworkConfig;
 
@@ -165,18 +165,12 @@ async fn spawn_consensus_actor(
     tx_event: TxEvent<MockContext>,
     span: &tracing::Span,
 ) -> ConsensusRef<MockContext> {
-    let value_payload = match cfg.test.value_payload {
-        malachitebft_config::ValuePayload::PartsOnly => ValuePayload::PartsOnly,
-        malachitebft_config::ValuePayload::ProposalOnly => ValuePayload::ProposalOnly,
-        malachitebft_config::ValuePayload::ProposalAndParts => ValuePayload::ProposalAndParts,
-    };
-
     let consensus_params = ConsensusParams {
         initial_height,
         initial_validator_set,
         address,
         threshold_params: Default::default(),
-        value_payload,
+        value_payload: ValuePayload::PartsOnly,
     };
 
     Consensus::spawn(
@@ -321,21 +315,20 @@ async fn spawn_host_actor(
     metrics: Metrics,
     span: &tracing::Span,
 ) -> HostRef<MockContext> {
-    let value_payload = match cfg.test.value_payload {
-        malachitebft_config::ValuePayload::PartsOnly => ValuePayload::PartsOnly,
-        malachitebft_config::ValuePayload::ProposalOnly => ValuePayload::ProposalOnly,
-        malachitebft_config::ValuePayload::ProposalAndParts => ValuePayload::ProposalAndParts,
-    };
+    if cfg.test.value_payload != config::ValuePayload::PartsOnly {
+        warn!(
+            "`value_payload` must be set to `PartsOnly` for Starknet app, ignoring current configuration `{:?}`",
+            cfg.test.value_payload
+        );
+    }
 
     let mock_params = StarknetParams {
-        value_payload,
         max_block_size: cfg.test.max_block_size,
         tx_size: cfg.test.tx_size,
         txs_per_part: cfg.test.txs_per_part,
         time_allowance_factor: cfg.test.time_allowance_factor,
         exec_time_per_tx: cfg.test.exec_time_per_tx,
         max_retain_blocks: cfg.test.max_retain_blocks,
-        // vote_extensions: cfg.test.vote_extensions,
     };
 
     let mock_host = StarknetHost::new(

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

@@ -15,9 +15,7 @@ use malachitebft_test_framework::HasTestRunner;
 use malachitebft_test_framework::{NodeRunner, TestNode};
 
 pub use malachitebft_test_framework::TestBuilder as GenTestBuilder;
-pub use malachitebft_test_framework::{
-    init_logging, EngineHandle, HandlerResult, Node, NodeId, TestParams,
-};
+pub use malachitebft_test_framework::{EngineHandle, HandlerResult, Node, NodeId, TestParams};
 
 use tempfile::TempDir;
 

code/crates/starknet/test/src/tests/mod.rs

@@ -1,6 +1,5 @@
 pub mod full_nodes;
 pub mod n3f0;
-pub mod n3f0_consensus_mode;
 pub mod n3f0_pubsub_protocol;
 pub mod n3f1;
 pub mod value_sync;