Add versioning to vote commitment calculations (#3584)

crates/example-types/src/node_types.rs

@@ -160,3 +160,132 @@ impl Versions for MarketplaceUpgradeTestVersions {
 
     type Marketplace = StaticVersion<0, 3>;
 }
+
+#[derive(Clone, Debug, Copy)]
+pub struct MarketplaceTestVersions {}
+
+impl Versions for MarketplaceTestVersions {
+    type Base = StaticVersion<0, 3>;
+    type Upgrade = StaticVersion<0, 3>;
+    const UPGRADE_HASH: [u8; 32] = [
+        1, 0, 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0,
+        0, 0,
+    ];
+
+    type Marketplace = StaticVersion<0, 3>;
+}
+
+#[cfg(test)]
+mod tests {
+    use committable::{Commitment, Committable};
+    use hotshot_types::{
+        message::UpgradeLock, simple_vote::VersionedVoteData,
+        traits::node_implementation::ConsensusTime,
+    };
+    use serde::{Deserialize, Serialize};
+
+    use crate::node_types::{MarketplaceTestVersions, NodeType, TestTypes, TestVersions};
+    #[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Hash, Eq)]
+    /// Dummy data used for test
+    struct TestData {
+        data: u64,
+    }
+
+    impl Committable for TestData {
+        fn commit(&self) -> Commitment<Self> {
+            committable::RawCommitmentBuilder::new("Test data")
+                .u64(self.data)
+                .finalize()
+        }
+    }
+
+    #[cfg_attr(async_executor_impl = "tokio", tokio::test(flavor = "multi_thread"))]
+    #[cfg_attr(async_executor_impl = "async-std", async_std::test)]
+    async fn test_versioned_commitment() {
+        let view = <TestTypes as NodeType>::Time::new(0);
+        let upgrade_lock = UpgradeLock::new();
+
+        let data = TestData { data: 10 };
+        let data_commitment: [u8; 32] = data.commit().into();
+
+        let versioned_data =
+            VersionedVoteData::<TestTypes, TestData, TestVersions>::new(data, view, &upgrade_lock)
+                .await
+                .unwrap();
+        let versioned_data_commitment: [u8; 32] = versioned_data.commit().into();
+
+        assert_eq!(versioned_data_commitment, data_commitment);
+    }
+
+    #[cfg_attr(async_executor_impl = "tokio", tokio::test(flavor = "multi_thread"))]
+    #[cfg_attr(async_executor_impl = "async-std", async_std::test)]
+    /// Test that the view number affects the commitment post-marketplace
+    async fn test_versioned_commitment_includes_view() {
+        let upgrade_lock = UpgradeLock::new();
+
+        let data = TestData { data: 10 };
+
+        let view_0 = <TestTypes as NodeType>::Time::new(0);
+        let view_1 = <TestTypes as NodeType>::Time::new(1);
+
+        let versioned_data_0 =
+            VersionedVoteData::<TestTypes, TestData, MarketplaceTestVersions>::new(
+                data,
+                view_0,
+                &upgrade_lock,
+            )
+            .await
+            .unwrap();
+        let versioned_data_1 =
+            VersionedVoteData::<TestTypes, TestData, MarketplaceTestVersions>::new(
+                data,
+                view_1,
+                &upgrade_lock,
+            )
+            .await
+            .unwrap();
+
+        let versioned_data_commitment_0: [u8; 32] = versioned_data_0.commit().into();
+        let versioned_data_commitment_1: [u8; 32] = versioned_data_1.commit().into();
+
+        assert!(
+            versioned_data_commitment_0 != versioned_data_commitment_1,
+            "left: {versioned_data_commitment_0:?}, right: {versioned_data_commitment_1:?}"
+        );
+    }
+
+    #[cfg_attr(async_executor_impl = "tokio", tokio::test(flavor = "multi_thread"))]
+    #[cfg_attr(async_executor_impl = "async-std", async_std::test)]
+    /// Test that the view number does not affect the commitment pre-marketplace
+    async fn test_versioned_commitment_excludes_view() {
+        let upgrade_lock = UpgradeLock::new();
+
+        let data = TestData { data: 10 };
+
+        let view_0 = <TestTypes as NodeType>::Time::new(0);
+        let view_1 = <TestTypes as NodeType>::Time::new(1);
+
+        let versioned_data_0 = VersionedVoteData::<TestTypes, TestData, TestVersions>::new(
+            data,
+            view_0,
+            &upgrade_lock,
+        )
+        .await
+        .unwrap();
+        let versioned_data_1 = VersionedVoteData::<TestTypes, TestData, TestVersions>::new(
+            data,
+            view_1,
+            &upgrade_lock,
+        )
+        .await
+        .unwrap();
+
+        let versioned_data_commitment_0: [u8; 32] = versioned_data_0.commit().into();
+        let versioned_data_commitment_1: [u8; 32] = versioned_data_1.commit().into();
+
+        assert!(
+            versioned_data_commitment_0 == versioned_data_commitment_1,
+            "left: {versioned_data_commitment_0:?}, right: {versioned_data_commitment_1:?}"
+        );
+    }
+}

crates/hotshot/src/tasks/mod.rs

@@ -206,9 +206,9 @@ pub fn add_network_event_task<
 pub async fn add_consensus_tasks<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>(
     handle: &mut SystemContextHandle<TYPES, I, V>,
 ) {
-    handle.add_task(ViewSyncTaskState::<TYPES, I>::create_from(handle).await);
+    handle.add_task(ViewSyncTaskState::<TYPES, I, V>::create_from(handle).await);
     handle.add_task(VidTaskState::<TYPES, I>::create_from(handle).await);
-    handle.add_task(DaTaskState::<TYPES, I>::create_from(handle).await);
+    handle.add_task(DaTaskState::<TYPES, I, V>::create_from(handle).await);
     handle.add_task(TransactionTaskState::<TYPES, I, V>::create_from(handle).await);
 
     // only spawn the upgrade task if we are actually configured to perform an upgrade.
@@ -619,6 +619,45 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES> + std::fmt::Debug, V: Version
     }
 }
 
+#[derive(Debug)]
+/// An `EventHandlerState` that modifies view number on the certificate of `DacSend` event to that of a future view
+pub struct DishonestDa {
+    /// How many times current node has been elected leader and sent Da Cert
+    pub total_da_certs_sent_from_node: u64,
+    /// Which proposals to be dishonest at
+    pub dishonest_at_da_cert_sent_numbers: HashSet<u64>,
+    /// When leader how many times we will send DacSend and increment view number
+    pub total_views_add_to_cert: u64,
+}
+
+#[async_trait]
+impl<TYPES: NodeType, I: NodeImplementation<TYPES> + std::fmt::Debug, V: Versions>
+    EventTransformerState<TYPES, I, V> for DishonestDa
+{
+    async fn recv_handler(&mut self, event: &HotShotEvent<TYPES>) -> Vec<HotShotEvent<TYPES>> {
+        vec![event.clone()]
+    }
+
+    async fn send_handler(&mut self, event: &HotShotEvent<TYPES>) -> Vec<HotShotEvent<TYPES>> {
+        if let HotShotEvent::DacSend(cert, sender) = event {
+            self.total_da_certs_sent_from_node += 1;
+            if self
+                .dishonest_at_da_cert_sent_numbers
+                .contains(&self.total_da_certs_sent_from_node)
+            {
+                let mut result = vec![HotShotEvent::DacSend(cert.clone(), sender.clone())];
+                for i in 1..=self.total_views_add_to_cert {
+                    let mut bad_cert = cert.clone();
+                    bad_cert.view_number = cert.view_number + i;
+                    result.push(HotShotEvent::DacSend(bad_cert, sender.clone()));
+                }
+                return result;
+            }
+        }
+        vec![event.clone()]
+    }
+}
+
 /// adds tasks for sending/receiving messages to/from the network.
 pub async fn add_network_tasks<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>(
     handle: &mut SystemContextHandle<TYPES, I, V>,

crates/hotshot/src/tasks/task_state.rs

@@ -130,7 +130,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
 
 #[async_trait]
 impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState<TYPES, I, V>
-    for DaTaskState<TYPES, I>
+    for DaTaskState<TYPES, I, V>
 {
     async fn create_from(handle: &SystemContextHandle<TYPES, I, V>) -> Self {
         Self {
@@ -145,13 +145,14 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
             private_key: handle.private_key().clone(),
             id: handle.hotshot.id,
             storage: Arc::clone(&handle.storage),
+            upgrade_lock: handle.hotshot.upgrade_lock.clone(),
         }
     }
 }
 
 #[async_trait]
 impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState<TYPES, I, V>
-    for ViewSyncTaskState<TYPES, I>
+    for ViewSyncTaskState<TYPES, I, V>
 {
     async fn create_from(handle: &SystemContextHandle<TYPES, I, V>) -> Self {
         let cur_view = handle.cur_view().await;
@@ -176,6 +177,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
             view_sync_timeout: handle.hotshot.config.view_sync_timeout,
             id: handle.hotshot.id,
             last_garbage_collected_view: TYPES::Time::new(0),
+            upgrade_lock: handle.hotshot.upgrade_lock.clone(),
         }
     }
 }

crates/task-impls/src/consensus/handlers.rs

@@ -183,6 +183,7 @@ pub async fn publish_proposal_from_commitment_and_metadata<TYPES: NodeType, V: V
         quorum_membership,
         public_key.clone(),
         OuterConsensus::new(Arc::clone(&consensus.inner_consensus)),
+        &upgrade_lock,
     )
     .await?;
 
@@ -323,13 +324,21 @@ pub(crate) async fn handle_quorum_proposal_recv<
         task_state.cur_view,
         &task_state.quorum_membership,
         &task_state.timeout_membership,
+        &task_state.upgrade_lock,
     )
+    .await
     .context("Failed to validate proposal view and attached certs")?;
 
     let view = proposal.data.view_number();
     let justify_qc = proposal.data.justify_qc.clone();
 
-    if !justify_qc.is_valid_cert(task_state.quorum_membership.as_ref()) {
+    if !justify_qc
+        .is_valid_cert(
+            task_state.quorum_membership.as_ref(),
+            &task_state.upgrade_lock,
+        )
+        .await
+    {
         let consensus = task_state.consensus.read().await;
         consensus.metrics.invalid_qc.update(1);
         bail!("Invalid justify_qc in proposal for view {}", *view);
@@ -370,6 +379,7 @@ pub(crate) async fn handle_quorum_proposal_recv<
             Arc::clone(&task_state.quorum_membership),
             OuterConsensus::new(Arc::clone(&task_state.consensus.inner_consensus)),
             task_state.public_key.clone(),
+            &task_state.upgrade_lock,
         )
         .await
         .ok(),
@@ -521,6 +531,7 @@ pub(crate) async fn handle_quorum_proposal_recv<
                 sender,
                 task_state.output_event_stream.clone(),
                 task_state.id,
+                task_state.upgrade_lock.clone(),
             )
             .map(AnyhowTracing::err_as_debug),
         ));
@@ -691,6 +702,7 @@ pub async fn update_state_and_vote_if_able<
     instance_state: Arc<TYPES::InstanceState>,
     vote_info: VoteInfo<TYPES, V>,
     id: u64,
+    upgrade_lock: &UpgradeLock<TYPES, V>,
 ) -> bool {
     use hotshot_types::simple_vote::QuorumVote;
 
@@ -754,6 +766,7 @@ pub async fn update_state_and_vote_if_able<
             Arc::clone(&quorum_membership),
             OuterConsensus::new(Arc::clone(&consensus.inner_consensus)),
             public_key.clone(),
+            upgrade_lock,
         )
         .await
         .ok(),
@@ -807,7 +820,10 @@ pub async fn update_state_and_vote_if_able<
     }
 
     // Validate the DAC.
-    let message = if cert.is_valid_cert(vote_info.da_membership.as_ref()) {
+    let message = if cert
+        .is_valid_cert(vote_info.da_membership.as_ref(), upgrade_lock)
+        .await
+    {
         // Validate the block payload commitment for non-genesis DAC.
         if cert.date().payload_commit != proposal.block_header.payload_commitment() {
             warn!(
@@ -823,7 +839,10 @@ pub async fn update_state_and_vote_if_able<
             view,
             &public_key,
             &vote_info.private_key,
-        ) {
+            &vote_info.upgrade_lock,
+        )
+        .await
+        {
             GeneralConsensusMessage::<TYPES>::Vote(vote)
         } else {
             error!("Unable to sign quorum vote!");