all done

EspressoSystems · Nov 28, 2023 · 179d8fc · 179d8fc
1 parent d439b5f
commit 179d8fc
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 56 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/task-impls/Cargo.toml b/crates/task-impls/Cargo.toml
@@ -26,6 +26,7 @@ rand_chacha = { workspace = true }
 hotshot-utils = { path = "../utils" }
 bincode = { workspace = true }
 bitvec = { workspace = true }
+sha2 = { workspace = true }
 
 [target.'cfg(all(async_executor_impl = "tokio"))'.dependencies]
 tokio = { workspace = true }

diff --git a/crates/task-impls/src/da.rs b/crates/task-impls/src/da.rs
@@ -30,6 +30,7 @@ use hotshot_types::{
     vote::HasViewNumber,
     vote::VoteAccumulator,
 };
+use sha2::{Digest, Sha256};
 
 use snafu::Snafu;
 use std::{collections::HashMap, marker::PhantomData, sync::Arc};
@@ -158,6 +159,15 @@ async fn vote_handle<TYPES: NodeType, I: NodeImplementation<TYPES>>(
     (None, state)
 }
 
+/// Helper function to calculate a sha256 hash
+/// Used primarily for signing and verifying the encoded transactions
+#[must_use]
+pub fn sha256_hash(data: &Vec<u8>) -> [u8; 32] {
+    let mut hasher = Sha256::new();
+    hasher.update(data);
+    hasher.finalize().into()
+}
+
 impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> + 'static>
     DATaskState<TYPES, I, A>
 {
@@ -196,6 +206,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                 }
 
                 let payload_commitment = vid_commitment(proposal.data.encoded_transactions.clone());
+                let encoded_transactions_hash = sha256_hash(&proposal.data.encoded_transactions);
 
                 // ED Is this the right leader?
                 let view_leader_key = self.da_membership.get_leader(view);
@@ -204,7 +215,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     return None;
                 }
 
-                if !view_leader_key.validate(&proposal.signature, payload_commitment.as_ref()) {
+                if !view_leader_key.validate(&proposal.signature, &encoded_transactions_hash) {
                     error!("Could not verify proposal.");
                     return None;
                 }
@@ -364,18 +375,17 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
 
                 return None;
             }
-            HotShotEvent::TransactionsSequenced(
-                encoded_transactions,
-                payload_commitment,
-                metadata,
-                view,
-            ) => {
+            HotShotEvent::TransactionsSequenced(encoded_transactions, metadata, view) => {
                 self.da_network
                     .inject_consensus_info(ConsensusIntentEvent::CancelPollForTransactions(*view))
                     .await;
 
+                // quick hash the encoded txns with sha256
+                let encoded_transactions_hash = sha256_hash(&encoded_transactions);
+
+                // sign the encoded transactions as opposed to the VID commitment
                 let signature =
-                    TYPES::SignatureKey::sign(&self.private_key, payload_commitment.as_ref());
+                    TYPES::SignatureKey::sign(&self.private_key, &encoded_transactions_hash);
                 let data: DAProposal<TYPES> = DAProposal {
                     encoded_transactions,
                     metadata: metadata.clone(),
@@ -422,7 +432,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
             HotShotEvent::DAProposalRecv(_, _)
                 | HotShotEvent::DAVoteRecv(_)
                 | HotShotEvent::Shutdown
-                | HotShotEvent::TransactionsSequenced(_, _, _, _)
+                | HotShotEvent::TransactionsSequenced(_, _, _)
                 | HotShotEvent::Timeout(_)
                 | HotShotEvent::ViewChange(_)
         )

diff --git a/crates/task-impls/src/events.rs b/crates/task-impls/src/events.rs
@@ -95,7 +95,6 @@ pub enum HotShotEvent<TYPES: NodeType> {
     /// Event when the transactions task has sequenced transactions. Contains the encoded transactions
     TransactionsSequenced(
         Vec<u8>,
-        VidCommitment,
         <TYPES::BlockPayload as BlockPayload>::Metadata,
         TYPES::Time,
     ),

diff --git a/crates/task-impls/src/transactions.rs b/crates/task-impls/src/transactions.rs
@@ -16,7 +16,6 @@ use hotshot_types::{
     consensus::Consensus,
     data::Leaf,
     traits::{
-        block_contents::vid_commitment,
         consensus_api::ConsensusApi,
         election::Membership,
         node_implementation::{NodeImplementation, NodeType},
@@ -228,28 +227,15 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     }
                 };
 
-                // calculate the payload commitment
-                // TODO never clone a block
-                // https://github.com/EspressoSystems/HotShot/issues/1858
-                let commitment = vid_commitment(encoded_transactions.clone());
-
                 // send the sequenced transactions to VID and DA tasks
                 self.event_stream
                     .publish(HotShotEvent::TransactionsSequenced(
                         encoded_transactions,
-                        commitment,
                         metadata.clone(),
                         view + 1,
                     ))
                     .await;
 
-                // send the commitment and metadata to consensus for timeout purposes
-                self.event_stream
-                    .publish(HotShotEvent::SendPayloadCommitmentAndMetadata(
-                        commitment, metadata,
-                    ))
-                    .await;
-
                 return None;
             }
             HotShotEvent::Shutdown => {

diff --git a/crates/task-impls/src/vid.rs b/crates/task-impls/src/vid.rs
@@ -345,12 +345,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     .await;
             }
 
-            HotShotEvent::TransactionsSequenced(
-                encoded_transactions,
-                commitment,
-                _metadata,
-                view_number,
-            ) => {
+            HotShotEvent::TransactionsSequenced(encoded_transactions, metadata, view_number) => {
                 // TODO <https://github.com/EspressoSystems/HotShot/issues/1686>
                 let srs = test_srs(NUM_STORAGE_NODES);
                 // TODO We are using constant numbers for now, but they will change as the quorum size
@@ -359,15 +354,24 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                 let vid = VidScheme::new(NUM_CHUNKS, NUM_STORAGE_NODES, &srs).unwrap();
                 let vid_disperse = vid.disperse(encoded_transactions.clone()).unwrap();
 
+                // send the commitment and metadata to consensus for block building
+                self.event_stream
+                    .publish(HotShotEvent::SendPayloadCommitmentAndMetadata(
+                        vid_disperse.commit,
+                        metadata,
+                    ))
+                    .await;
+
+                // send the block to the VID dispersal function
                 self.event_stream
                     .publish(HotShotEvent::BlockReady(
                         VidDisperse {
                             view_number,
-                            payload_commitment: commitment,
+                            payload_commitment: vid_disperse.commit,
                             shares: vid_disperse.shares,
                             common: vid_disperse.common,
                         },
-                        commitment,
+                        vid_disperse.commit,
                         view_number,
                     ))
                     .await;
@@ -447,7 +451,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                 | HotShotEvent::VidDisperseRecv(_, _)
                 | HotShotEvent::VidVoteRecv(_)
                 | HotShotEvent::VidCertRecv(_)
-                | HotShotEvent::TransactionsSequenced(_, _, _, _)
+                | HotShotEvent::TransactionsSequenced(_, _, _)
                 | HotShotEvent::BlockReady(_, _, _)
                 | HotShotEvent::ViewChange(_)
         )

diff --git a/crates/testing/tests/da_task.rs b/crates/testing/tests/da_task.rs
@@ -1,5 +1,5 @@
 use hotshot::{types::SignatureKey, HotShotConsensusApi};
-use hotshot_task_impls::events::HotShotEvent;
+use hotshot_task_impls::{da::sha256_hash, events::HotShotEvent};
 use hotshot_testing::node_types::{MemoryImpl, TestTypes};
 use hotshot_types::{
     block_impl::VIDTransaction,
@@ -35,9 +35,10 @@ async fn test_da_task() {
     let transactions = vec![VIDTransaction(vec![0])];
     let encoded_transactions = VIDTransaction::encode(transactions.clone()).unwrap();
     let payload_commitment = vid_commitment(encoded_transactions.clone());
+    let encoded_transactions_hash = sha256_hash(&encoded_transactions);
 
     let signature =
-        <TestTypes as NodeType>::SignatureKey::sign(api.private_key(), payload_commitment.as_ref());
+        <TestTypes as NodeType>::SignatureKey::sign(api.private_key(), &encoded_transactions_hash);
     let proposal = DAProposal {
         encoded_transactions: encoded_transactions.clone(),
         metadata: (),
@@ -61,7 +62,6 @@ async fn test_da_task() {
     input.push(HotShotEvent::ViewChange(ViewNumber::new(2)));
     input.push(HotShotEvent::TransactionsSequenced(
         encoded_transactions.clone(),
-        payload_commitment,
         (),
         ViewNumber::new(2),
     ));
@@ -71,12 +71,7 @@ async fn test_da_task() {
 
     output.insert(HotShotEvent::ViewChange(ViewNumber::new(1)), 1);
     output.insert(
-        HotShotEvent::TransactionsSequenced(
-            encoded_transactions,
-            payload_commitment,
-            (),
-            ViewNumber::new(2),
-        ),
+        HotShotEvent::TransactionsSequenced(encoded_transactions, (), ViewNumber::new(2)),
         1,
     );
     output.insert(HotShotEvent::DAProposalSend(message.clone(), pub_key), 1);

diff --git a/crates/testing/tests/network_task.rs b/crates/testing/tests/network_task.rs
@@ -74,7 +74,6 @@ async fn test_network_task() {
     input.push(HotShotEvent::ViewChange(ViewNumber::new(1)));
     input.push(HotShotEvent::TransactionsSequenced(
         encoded_transactions.clone(),
-        payload_commitment,
         (),
         ViewNumber::new(2),
     ));
@@ -101,12 +100,7 @@ async fn test_network_task() {
         2, // 2 occurrences: 1 from `input`, 1 from the DA task
     );
     output.insert(
-        HotShotEvent::TransactionsSequenced(
-            encoded_transactions,
-            payload_commitment,
-            (),
-            ViewNumber::new(2),
-        ),
+        HotShotEvent::TransactionsSequenced(encoded_transactions, (), ViewNumber::new(2)),
         2,
     );
     output.insert(

diff --git a/crates/testing/tests/vid_task.rs b/crates/testing/tests/vid_task.rs
@@ -72,7 +72,6 @@ async fn test_vid_task() {
     input.push(HotShotEvent::ViewChange(ViewNumber::new(2)));
     input.push(HotShotEvent::TransactionsSequenced(
         encoded_transactions.clone(),
-        payload_commitment,
         (),
         ViewNumber::new(2),
     ));
@@ -87,12 +86,7 @@ async fn test_vid_task() {
 
     output.insert(HotShotEvent::ViewChange(ViewNumber::new(1)), 1);
     output.insert(
-        HotShotEvent::TransactionsSequenced(
-            encoded_transactions,
-            payload_commitment,
-            (),
-            ViewNumber::new(2),
-        ),
+        HotShotEvent::TransactionsSequenced(encoded_transactions, (), ViewNumber::new(2)),
         1,
     );
 
@@ -101,6 +95,10 @@ async fn test_vid_task() {
         2,
     );
 
+    output.insert(
+        HotShotEvent::SendPayloadCommitmentAndMetadata(payload_commitment, ()),
+        1,
+    );
     output.insert(
         HotShotEvent::VidDisperseSend(vid_proposal.clone(), pub_key),
         2, // 2 occurrences: 1 from `input`, 1 from the DA task