Add versioning to vote commitment calculations

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.

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!");

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

@@ -52,7 +52,8 @@ use crate::{
 pub(crate) mod handlers;
 
 /// Alias for Optional type for Vote Collectors
-type VoteCollectorOption<TYPES, VOTE, CERT> = Option<VoteCollectionTaskState<TYPES, VOTE, CERT>>;
+type VoteCollectorOption<TYPES, VOTE, CERT, V> =
+    Option<VoteCollectionTaskState<TYPES, VOTE, CERT, V>>;
 
 /// The state for the consensus task.  Contains all of the information for the implementation
 /// of consensus
@@ -92,11 +93,11 @@ pub struct ConsensusTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V:
 
     /// Current Vote collection task, with it's view.
     pub vote_collector:
-        RwLock<VoteCollectorOption<TYPES, QuorumVote<TYPES>, QuorumCertificate<TYPES>>>,
+        RwLock<VoteCollectorOption<TYPES, QuorumVote<TYPES>, QuorumCertificate<TYPES>, V>>,
 
     /// Current timeout vote collection task with its view
     pub timeout_vote_collector:
-        RwLock<VoteCollectorOption<TYPES, TimeoutVote<TYPES>, TimeoutCertificate<TYPES>>>,
+        RwLock<VoteCollectorOption<TYPES, TimeoutVote<TYPES>, TimeoutCertificate<TYPES>, V>>,
 
     /// timeout task handle
     pub timeout_task: JoinHandle<()>,
@@ -248,6 +249,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ConsensusTaskSt
         let instance_state = Arc::clone(&self.instance_state);
         let id = self.id;
         let handle = async_spawn(async move {
+            let upgrade_lock = upgrade.clone();
             update_state_and_vote_if_able::<TYPES, I, V>(
                 view,
                 proposal,
@@ -264,6 +266,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ConsensusTaskSt
                     event_receiver,
                 },
                 id,
+                &upgrade_lock,
             )
             .await;
         });
@@ -333,11 +336,12 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ConsensusTaskSt
                         view: vote.view_number(),
                         id: self.id,
                     };
-                    *collector = create_vote_accumulator::<
-                        TYPES,
-                        QuorumVote<TYPES>,
-                        QuorumCertificate<TYPES>,
-                    >(&info, event, &event_sender)
+                    *collector = create_vote_accumulator(
+                        &info,
+                        event,
+                        &event_sender,
+                        self.upgrade_lock.clone(),
+                    )
                     .await;
                 } else {
                     let result = collector
@@ -372,11 +376,12 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ConsensusTaskSt
                         view: vote.view_number(),
                         id: self.id,
                     };
-                    *collector = create_vote_accumulator::<
-                        TYPES,
-                        TimeoutVote<TYPES>,
-                        TimeoutCertificate<TYPES>,
-                    >(&info, event, &event_sender)
+                    *collector = create_vote_accumulator(
+                        &info,
+                        event,
+                        &event_sender,
+                        self.upgrade_lock.clone(),
+                    )
                     .await;
                 } else {
                     let result = collector
@@ -569,7 +574,10 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ConsensusTaskSt
                     view,
                     &self.public_key,
                     &self.private_key,
-                ) else {
+                    &self.upgrade_lock,
+                )
+                .await
+                else {
                     error!("Failed to sign TimeoutData!");
                     return;
                 };
@@ -667,7 +675,10 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ConsensusTaskSt
                 }
             }
             HotShotEvent::ViewSyncFinalizeCertificate2Recv(certificate) => {
-                if !certificate.is_valid_cert(self.quorum_membership.as_ref()) {
+                if !certificate
+                    .is_valid_cert(self.quorum_membership.as_ref(), &self.upgrade_lock)
+                    .await
+                {
                     error!(
                         "View Sync Finalize certificate {:?} was invalid",
                         certificate.date()