Merge pull request #2221 from EspressoSystems/keyao/ci-task-unit-tests

Fix network and consensus unit tests

crates/hotshot/src/traits/storage/memory_storage.rs

@@ -111,16 +111,13 @@ mod test {
     use super::*;
     use commit::Committable;
     use hotshot_testing::{
-        block_types::{TestBlockHeader, TestBlockPayload},
+        block_types::{genesis_vid_commitment, TestBlockHeader, TestBlockPayload},
         node_types::TestTypes,
     };
     use hotshot_types::{
         data::{fake_commitment, genesis_proposer_id, Leaf},
         simple_certificate::QuorumCertificate,
-        traits::{
-            block_contents::genesis_vid_commitment, node_implementation::NodeType,
-            state::ConsensusTime,
-        },
+        traits::{node_implementation::NodeType, state::ConsensusTime},
     };
     use std::marker::PhantomData;
     use tracing::instrument;

crates/task-impls/src/harness.rs

@@ -98,6 +98,7 @@ pub fn handle_event<TYPES: NodeType>(
         state.expected_output.contains_key(&event),
         "Got an unexpected event: {event:?}",
     );
+
     let num_expected = state.expected_output.get_mut(&event).unwrap();
     if *num_expected == 1 {
         state.expected_output.remove(&event);
@@ -108,5 +109,6 @@ pub fn handle_event<TYPES: NodeType>(
     if state.expected_output.is_empty() {
         return (Some(HotShotTaskCompleted::ShutDown), state);
     }
+
     (None, state)
 }

crates/testing/src/block_types.rs

@@ -5,9 +5,9 @@ use std::{
 
 use commit::{Commitment, Committable, RawCommitmentBuilder};
 use hotshot_types::{
-    data::{BlockError, VidCommitment},
+    data::{BlockError, VidCommitment, VidScheme, VidSchemeTrait},
     traits::{
-        block_contents::{genesis_vid_commitment, BlockHeader, Transaction},
+        block_contents::{vid_commitment, BlockHeader, Transaction},
         state::TestableBlock,
         BlockPayload,
     },
@@ -157,6 +157,16 @@ impl BlockPayload for TestBlockPayload {
     }
 }
 
+/// Computes the (empty) genesis VID commitment
+/// The number of storage nodes does not do anything, unless in the future we add fake transactions
+/// to the genesis payload.
+///
+/// In that case, the payloads may mismatch and cause problems.
+#[must_use]
+pub fn genesis_vid_commitment() -> <VidScheme as VidSchemeTrait>::Commit {
+    vid_commitment(&vec![], 8)
+}
+
 /// A [`BlockHeader`] that commits to [`TestBlockPayload`].
 #[derive(PartialEq, Eq, Hash, Clone, Debug, Deserialize, Serialize)]
 pub struct TestBlockHeader {

crates/testing/tests/consensus_task.rs

@@ -85,7 +85,6 @@ async fn build_vote(
     tokio::test(flavor = "multi_thread", worker_threads = 2)
 )]
 #[cfg_attr(async_executor_impl = "async-std", async_std::test)]
-#[ignore]
 async fn test_consensus_task() {
     use hotshot_task_impls::harness::run_harness;
     use hotshot_testing::task_helpers::build_system_handle;

crates/testing/tests/network_task.rs

@@ -8,6 +8,7 @@ use hotshot_types::{
     data::{DAProposal, VidSchemeTrait, ViewNumber},
     traits::{consensus_api::ConsensusApi, state::ConsensusTime},
 };
+use sha2::{Digest, Sha256};
 use std::{collections::HashMap, marker::PhantomData};
 
 #[cfg(test)]
@@ -16,10 +17,9 @@ use std::{collections::HashMap, marker::PhantomData};
     tokio::test(flavor = "multi_thread", worker_threads = 2)
 )]
 #[cfg_attr(async_executor_impl = "async-std", async_std::test)]
-#[ignore]
 async fn test_network_task() {
     use hotshot_task_impls::harness::run_harness;
-    use hotshot_testing::{block_types::TestTransaction, task_helpers::build_system_handle};
+    use hotshot_testing::task_helpers::build_system_handle;
     use hotshot_types::{data::VidDisperse, message::Proposal};
 
     async_compatibility_layer::logging::setup_logging();
@@ -35,38 +35,44 @@ async fn test_network_task() {
     // quorum membership for VID share distribution
     let quorum_membership = handle.hotshot.inner.memberships.quorum_membership.clone();
 
-    let vid = vid_init::<TestTypes>(quorum_membership.clone(), ViewNumber::new(0));
-    let transactions = vec![TestTransaction(vec![0])];
-    let encoded_transactions = TestTransaction::encode(transactions.clone()).unwrap();
+    let encoded_transactions = Vec::new();
+    let encoded_transactions_hash = Sha256::digest(&encoded_transactions);
+    let da_signature =
+        <TestTypes as hotshot_types::traits::node_implementation::NodeType>::SignatureKey::sign(
+            api.private_key(),
+            &encoded_transactions_hash,
+        );
+    let vid = vid_init::<TestTypes>(quorum_membership.clone(), ViewNumber::new(2));
     let vid_disperse = vid.disperse(&encoded_transactions).unwrap();
     let payload_commitment = vid_disperse.commit;
-    let signature =
+    let vid_signature =
         <TestTypes as hotshot_types::traits::node_implementation::NodeType>::SignatureKey::sign(
             api.private_key(),
             payload_commitment.as_ref(),
         );
+
     let da_proposal = Proposal {
         data: DAProposal {
             encoded_transactions: encoded_transactions.clone(),
             metadata: (),
             view_number: ViewNumber::new(2),
         },
-        signature,
+        signature: da_signature,
         _pd: PhantomData,
     };
     let quorum_proposal = build_quorum_proposal(&handle, priv_key, 2).await;
 
-    let da_vid_disperse_inner = VidDisperse::from_membership(
+    let vid_disperse_inner = VidDisperse::from_membership(
         da_proposal.data.view_number,
         vid_disperse,
         &quorum_membership.clone().into(),
     );
 
     // TODO for now reuse the same block payload commitment and signature as DA committee
     // https://github.com/EspressoSystems/jellyfish/issues/369
-    let da_vid_disperse = Proposal {
-        data: da_vid_disperse_inner.clone(),
-        signature: da_proposal.signature.clone(),
+    let vid_proposal = Proposal {
+        data: vid_disperse_inner.clone(),
+        signature: vid_signature,
         _pd: PhantomData,
     };
 
@@ -75,47 +81,41 @@ async fn test_network_task() {
     let mut output = HashMap::new();
 
     input.push(HotShotEvent::ViewChange(ViewNumber::new(1)));
+    input.push(HotShotEvent::ViewChange(ViewNumber::new(2)));
     input.push(HotShotEvent::TransactionsSequenced(
         encoded_transactions.clone(),
         (),
         ViewNumber::new(2),
     ));
     input.push(HotShotEvent::BlockReady(
-        da_vid_disperse_inner.clone(),
+        vid_disperse_inner.clone(),
         ViewNumber::new(2),
     ));
     input.push(HotShotEvent::DAProposalSend(da_proposal.clone(), pub_key));
-    input.push(HotShotEvent::VidDisperseSend(
-        da_vid_disperse.clone(),
-        pub_key,
-    ));
+    input.push(HotShotEvent::VidDisperseSend(vid_proposal.clone(), pub_key));
     input.push(HotShotEvent::QuorumProposalSend(
         quorum_proposal.clone(),
         pub_key,
     ));
-    input.push(HotShotEvent::ViewChange(ViewNumber::new(2)));
     input.push(HotShotEvent::Shutdown);
 
     output.insert(HotShotEvent::ViewChange(ViewNumber::new(1)), 2);
-    output.insert(
-        HotShotEvent::DAProposalSend(da_proposal.clone(), pub_key),
-        2, // 2 occurrences: 1 from `input`, 1 from the DA task
-    );
     output.insert(
         HotShotEvent::TransactionsSequenced(encoded_transactions, (), ViewNumber::new(2)),
-        2,
+        2, // 2 occurrences: 1 from `input`, 1 from the transactions task
     );
     output.insert(
-        HotShotEvent::VidDisperseRecv(da_vid_disperse.clone(), pub_key),
-        1,
+        HotShotEvent::BlockReady(vid_disperse_inner, ViewNumber::new(2)),
+        2, // 2 occurrences: 1 from `input`, 1 from the VID task
     );
     output.insert(
-        HotShotEvent::VidDisperseSend(da_vid_disperse, pub_key),
+        HotShotEvent::DAProposalSend(da_proposal.clone(), pub_key),
         2, // 2 occurrences: 1 from `input`, 1 from the DA task
     );
-    output.insert(HotShotEvent::Timeout(ViewNumber::new(1)), 1);
-    output.insert(HotShotEvent::Timeout(ViewNumber::new(2)), 1);
-
+    output.insert(
+        HotShotEvent::VidDisperseSend(vid_proposal.clone(), pub_key),
+        2, // 2 occurrences: 1 from `input`, 1 from the VID task
+    );
     // Only one output from the input.
     // The consensus task will fail to send a second proposal, like the DA task does, due to the
     // view number check in `publish_proposal_if_able` in consensus.rs, and we will see an error in
@@ -125,20 +125,17 @@ async fn test_network_task() {
         HotShotEvent::QuorumProposalSend(quorum_proposal.clone(), pub_key),
         1,
     );
+    output.insert(HotShotEvent::ViewChange(ViewNumber::new(2)), 2);
     output.insert(
         HotShotEvent::SendPayloadCommitmentAndMetadata(payload_commitment, ()),
         1,
     );
-    output.insert(
-        HotShotEvent::BlockReady(da_vid_disperse_inner, ViewNumber::new(2)),
-        2,
-    );
-    output.insert(HotShotEvent::DAProposalRecv(da_proposal, pub_key), 1);
     output.insert(
         HotShotEvent::QuorumProposalRecv(quorum_proposal, pub_key),
         1,
     );
-    output.insert(HotShotEvent::ViewChange(ViewNumber::new(2)), 2);
+    output.insert(HotShotEvent::VidDisperseRecv(vid_proposal, pub_key), 1);
+    output.insert(HotShotEvent::DAProposalRecv(da_proposal, pub_key), 1);
     output.insert(HotShotEvent::Shutdown, 1);
 
     let build_fn = |task_runner, _| async { task_runner };

crates/types/src/traits/block_contents.rs

@@ -13,13 +13,6 @@ use std::{
     hash::Hash,
 };
 
-// TODO <https://github.com/EspressoSystems/HotShot/issues/1693>
-/// Number of storage nodes for VID initiation.
-pub const NUM_STORAGE_NODES: usize = 8;
-// TODO <https://github.com/EspressoSystems/HotShot/issues/1693>
-/// Number of chunks for VID initiation.
-pub const NUM_CHUNKS: usize = 8;
-
 /// Abstraction over any type of transaction. Used by [`BlockPayload`].
 pub trait Transaction:
     Clone + Serialize + DeserializeOwned + Debug + PartialEq + Eq + Sync + Send + Committable + Hash
@@ -97,16 +90,6 @@ pub fn vid_commitment(
     vid.commit_only(encoded_transactions).unwrap()
 }
 
-/// Computes the (empty) genesis VID commitment
-/// The number of storage nodes does not do anything, unless in the future we add fake transactions
-/// to the genesis payload.
-///
-/// In that case, the payloads may mismatch and cause problems.
-#[must_use]
-pub fn genesis_vid_commitment() -> <VidScheme as VidSchemeTrait>::Commit {
-    vid_commitment(&vec![], 8)
-}
-
 /// Header of a block, which commits to a [`BlockPayload`].
 pub trait BlockHeader:
     Serialize + Clone + Debug + Hash + PartialEq + Eq + Send + Sync + DeserializeOwned + Committable