Merge pull request #2112 from EspressoSystems/jr/nuke_encoded

[Tech Debt] Nuke Encoded{Signature,PublicKey}

constants/src/lib.rs

@@ -1,8 +1,5 @@
 //! configurable constants for hotshot
 
-/// the ID of the genesis block proposer
-pub const GENESIS_PROPOSER_ID: [u8; 2] = [4, 2];
-
 /// the number of views to gather information for ahead of time
 pub const LOOK_AHEAD: u64 = 5;
 

hotshot-signature-key/src/bn254.rs

@@ -1,5 +1,5 @@
 //! Demonstration implementation of the [`SignatureKey`] trait using BN254
-use hotshot_types::traits::signature_key::{EncodedPublicKey, EncodedSignature, SignatureKey};
+use hotshot_types::traits::signature_key::{EncodedPublicKey, SignatureKey};
 /// `BLSPrivKey` implementation
 mod bn254_priv;
 /// `BLSPubKey` implementation

hotshot-signature-key/src/bn254/bn254_pub.rs

@@ -1,4 +1,4 @@
-use super::{BLSPrivKey, EncodedPublicKey, EncodedSignature, SignatureKey};
+use super::{BLSPrivKey, EncodedPublicKey, SignatureKey};
 use bincode::Options;
 use bitvec::prelude::*;
 use blake3::traits::digest::generic_array::GenericArray;
@@ -8,6 +8,7 @@ use hotshot_qc::bit_vector_old::{
 };
 use hotshot_types::traits::qc::QuorumCertificate;
 use hotshot_utils::bincode::bincode_opts;
+use jf_primitives::errors::PrimitivesError;
 use jf_primitives::signatures::{
     bls_over_bn254::{BLSOverBN254CurveSignatureScheme, VerKey},
     SignatureScheme,
@@ -51,47 +52,28 @@ impl SignatureKey for BLSPubKey {
     type PureAssembledSignatureType =
         <BLSOverBN254CurveSignatureScheme as SignatureScheme>::Signature;
     type QCType = (Self::PureAssembledSignatureType, BitVec);
+    type SignError = PrimitivesError;
 
     #[instrument(skip(self))]
-    fn validate(&self, signature: &EncodedSignature, data: &[u8]) -> bool {
+    fn validate(&self, signature: &Self::PureAssembledSignatureType, data: &[u8]) -> bool {
         let ver_key = self.pub_key;
-        let x: Result<<BLSOverBN254CurveSignatureScheme as SignatureScheme>::Signature, _> =
-            bincode_opts().deserialize(&signature.0);
-        match x {
-            Ok(s) => {
-                // This is the validation for QC partial signature before append().
-                let generic_msg: &GenericArray<u8, U32> = GenericArray::from_slice(data);
-                BLSOverBN254CurveSignatureScheme::verify(&(), &ver_key, generic_msg, &s).is_ok()
-            }
-            Err(_) => false,
-        }
+
+        // This is the validation for QC partial signature before append().
+        let generic_msg: &GenericArray<u8, U32> = GenericArray::from_slice(data);
+        BLSOverBN254CurveSignatureScheme::verify(&(), &ver_key, generic_msg, signature).is_ok()
     }
 
-    fn sign(sk: &Self::PrivateKey, data: &[u8]) -> EncodedSignature {
+    fn sign(
+        sk: &Self::PrivateKey,
+        data: &[u8],
+    ) -> Result<Self::PureAssembledSignatureType, Self::SignError> {
         let generic_msg = GenericArray::from_slice(data);
-        let agg_signature_wrap = BitVectorQC::<BLSOverBN254CurveSignatureScheme>::sign(
+        BitVectorQC::<BLSOverBN254CurveSignatureScheme>::sign(
             &(),
             generic_msg,
             &sk.priv_key,
             &mut rand::thread_rng(),
-        );
-        match agg_signature_wrap {
-            Ok(agg_signature) => {
-                // Convert the signature to bytes and return
-                let bytes = bincode_opts().serialize(&agg_signature);
-                match bytes {
-                    Ok(bytes) => EncodedSignature(bytes),
-                    Err(e) => {
-                        warn!(?e, "Failed to serialize signature in sign()");
-                        EncodedSignature(vec![])
-                    }
-                }
-            }
-            Err(e) => {
-                warn!(?e, "Failed to sign");
-                EncodedSignature(vec![])
-            }
-        }
+        )
     }
 
     fn from_private(private_key: &Self::PrivateKey) -> Self {
@@ -164,4 +146,14 @@ impl SignatureKey for BLSPubKey {
         BitVectorQC::<BLSOverBN254CurveSignatureScheme>::assemble(real_qc_pp, signers, sigs)
             .expect("this assembling shouldn't fail")
     }
+
+    fn genesis_proposer_pk() -> Self {
+        use jf_primitives::signatures::bls_over_bn254::KeyPair;
+        use rand::rngs::mock::StepRng;
+        let mut my_rng = StepRng::new(42, 1337);
+        let kp = KeyPair::generate(&mut my_rng);
+        BLSPubKey {
+            pub_key: kp.ver_key(),
+        }
+    }
 }

hotshot/src/traits/storage/memory_storage.rs

@@ -115,9 +115,11 @@ mod test {
         node_types::TestTypes,
     };
     use hotshot_types::{
-        data::{fake_commitment, genesis_proposer_id, Leaf},
+        data::{fake_commitment, Leaf},
         simple_certificate::QuorumCertificate,
-        traits::{node_implementation::NodeType, state::ConsensusTime},
+        traits::{
+            node_implementation::NodeType, signature_key::SignatureKey, state::ConsensusTime,
+        },
     };
     use std::marker::PhantomData;
     use tracing::instrument;
@@ -146,7 +148,7 @@ mod test {
             Some(payload),
             dummy_leaf_commit,
             Vec::new(),
-            genesis_proposer_id(),
+            <<TestTypes as NodeType>::SignatureKey as SignatureKey>::genesis_proposer_pk(),
         )
     }
 

task-impls/src/consensus.rs

@@ -221,16 +221,19 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     block_payload: None,
                     rejected: Vec::new(),
                     timestamp: time::OffsetDateTime::now_utc().unix_timestamp_nanos(),
-                    proposer_id: self.quorum_membership.get_leader(view).to_bytes(),
+                    proposer_id: self.quorum_membership.get_leader(view),
                 };
-                let vote = QuorumVote::<TYPES>::create_signed_vote(
+                let Ok(vote) = QuorumVote::<TYPES>::create_signed_vote(
                     QuorumData {
                         leaf_commit: leaf.commit(),
                     },
                     view,
                     &self.public_key,
                     &self.private_key,
-                );
+                ) else {
+                    error!("Failed to sign QuorumData!");
+                    return false;
+                };
 
                 let message = GeneralConsensusMessage::<TYPES>::Vote(vote);
 
@@ -305,7 +308,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     block_payload: None,
                     rejected: Vec::new(),
                     timestamp: time::OffsetDateTime::now_utc().unix_timestamp_nanos(),
-                    proposer_id: self.quorum_membership.get_leader(view).to_bytes(),
+                    proposer_id: self.quorum_membership.get_leader(view),
                 };
 
                 // Validate the DAC.
@@ -318,15 +321,19 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                         error!("Block payload commitment does not equal da cert payload commitment. View = {}", *view);
                         return false;
                     }
-                    let vote = QuorumVote::<TYPES>::create_signed_vote(
+                    if let Ok(vote) = QuorumVote::<TYPES>::create_signed_vote(
                         QuorumData {
                             leaf_commit: leaf.commit(),
                         },
                         view,
                         &self.public_key,
                         &self.private_key,
-                    );
-                    GeneralConsensusMessage::<TYPES>::Vote(vote)
+                    ) {
+                        GeneralConsensusMessage::<TYPES>::Vote(vote)
+                    } else {
+                        error!("Unable to sign quorum vote!");
+                        return false;
+                    }
                 } else {
                     error!(
                         "Invalid DAC in proposal! Skipping proposal. {:?} cur view is: {:?}",
@@ -525,7 +532,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                         block_payload: None,
                         rejected: Vec::new(),
                         timestamp: time::OffsetDateTime::now_utc().unix_timestamp_nanos(),
-                        proposer_id: sender.to_bytes(),
+                        proposer_id: sender,
                     };
 
                     let mut consensus = RwLockUpgradableReadGuard::upgrade(consensus).await;
@@ -550,7 +557,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     block_payload: None,
                     rejected: Vec::new(),
                     timestamp: time::OffsetDateTime::now_utc().unix_timestamp_nanos(),
-                    proposer_id: sender.to_bytes(),
+                    proposer_id: sender,
                 };
                 let leaf_commitment = leaf.commit();
 
@@ -1024,12 +1031,15 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     .inject_consensus_info(ConsensusIntentEvent::CancelPollForProposal(*view))
                     .await;
 
-                let vote = TimeoutVote::create_signed_vote(
+                let Ok(vote) = TimeoutVote::create_signed_vote(
                     TimeoutData { view },
                     view,
                     &self.public_key,
                     &self.private_key,
-                );
+                ) else {
+                    error!("Failed to sign TimeoutData!");
+                    return;
+                };
 
                 self.event_stream
                     .publish(HotShotEvent::TimeoutVoteSend(vote))
@@ -1157,10 +1167,15 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                 block_payload: None,
                 rejected: vec![],
                 timestamp: time::OffsetDateTime::now_utc().unix_timestamp_nanos(),
-                proposer_id: self.api.public_key().to_bytes(),
+                proposer_id: self.api.public_key().clone(),
             };
 
-            let signature = TYPES::SignatureKey::sign(&self.private_key, leaf.commit().as_ref());
+            let Ok(signature) =
+                TYPES::SignatureKey::sign(&self.private_key, leaf.commit().as_ref())
+            else {
+                error!("Failed to sign leaf.commit()!");
+                return false;
+            };
             // TODO: DA cert is sent as part of the proposal here, we should split this out so we don't have to wait for it.
             let proposal = QuorumProposal {
                 block_header: leaf.block_header.clone(),

task-impls/src/da.rs

@@ -149,14 +149,17 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     return None;
                 }
                 // Generate and send vote
-                let vote = DAVote::create_signed_vote(
+                let Ok(vote) = DAVote::create_signed_vote(
                     DAData {
                         payload_commit: payload_commitment,
                     },
                     view,
                     &self.public_key,
                     &self.private_key,
-                );
+                ) else {
+                    error!("Failed to sign DA Vote!");
+                    return None;
+                };
 
                 // ED Don't think this is necessary?
                 // self.cur_view = view;
@@ -275,8 +278,13 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                 let encoded_transactions_hash = Sha256::digest(&encoded_transactions);
 
                 // sign the encoded transactions as opposed to the VID commitment
-                let signature =
-                    TYPES::SignatureKey::sign(&self.private_key, &encoded_transactions_hash);
+                let Ok(signature) =
+                    TYPES::SignatureKey::sign(&self.private_key, &encoded_transactions_hash)
+                else {
+                    error!("Failed to sign block payload!");
+                    return None;
+                };
+
                 let data: DAProposal<TYPES> = DAProposal {
                     encoded_transactions,
                     metadata: metadata.clone(),

task-impls/src/vid.rs

@@ -112,14 +112,17 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
             }
 
             HotShotEvent::BlockReady(vid_disperse, view_number) => {
+                let Ok(signature) =
+                    TYPES::SignatureKey::sign(&self.private_key, &vid_disperse.payload_commitment)
+                else {
+                    error!("VID: failed to sign dispersal payload");
+                    return None;
+                };
                 debug!("publishing VID disperse for view {}", *view_number);
                 self.event_stream
                     .publish(HotShotEvent::VidDisperseSend(
                         Proposal {
-                            signature: TYPES::SignatureKey::sign(
-                                &self.private_key,
-                                &vid_disperse.payload_commitment,
-                            ),
+                            signature,
                             data: vid_disperse,
                             _pd: PhantomData,
                         },

task-impls/src/view_sync.rs

@@ -585,15 +585,18 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     self.relay = certificate.get_data().relay;
                 }
 
-                let vote = ViewSyncCommitVote::<TYPES>::create_signed_vote(
+                let Ok(vote) = ViewSyncCommitVote::<TYPES>::create_signed_vote(
                     ViewSyncCommitData {
                         relay: certificate.get_data().relay,
                         round: self.next_view,
                     },
                     self.next_view,
                     &self.public_key,
                     &self.private_key,
-                );
+                ) else {
+                    error!("Failed to sign ViewSyncCommitData!");
+                    return (None, self);
+                };
                 let message = GeneralConsensusMessage::<TYPES>::ViewSyncCommitVote(vote);
 
                 if let GeneralConsensusMessage::ViewSyncCommitVote(vote) = message {
@@ -650,15 +653,18 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     self.relay = certificate.get_data().relay;
                 }
 
-                let vote = ViewSyncFinalizeVote::<TYPES>::create_signed_vote(
+                let Ok(vote) = ViewSyncFinalizeVote::<TYPES>::create_signed_vote(
                     ViewSyncFinalizeData {
                         relay: certificate.get_data().relay,
                         round: self.next_view,
                     },
                     self.next_view,
                     &self.public_key,
                     &self.private_key,
-                );
+                ) else {
+                    error!("Failed to sign view sync finalized vote!");
+                    return (None, self);
+                };
                 let message = GeneralConsensusMessage::<TYPES>::ViewSyncFinalizeVote(vote);
 
                 if let GeneralConsensusMessage::ViewSyncFinalizeVote(vote) = message {
@@ -765,15 +771,18 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     return (None, self);
                 }
 
-                let vote = ViewSyncPreCommitVote::<TYPES>::create_signed_vote(
+                let Ok(vote) = ViewSyncPreCommitVote::<TYPES>::create_signed_vote(
                     ViewSyncPreCommitData {
                         relay: 0,
                         round: view_number,
                     },
                     view_number,
                     &self.public_key,
                     &self.private_key,
-                );
+                ) else {
+                    error!("Failed to sign pre commit vote!");
+                    return (None, self);
+                };
                 let message = GeneralConsensusMessage::<TYPES>::ViewSyncPreCommitVote(vote);
 
                 if let GeneralConsensusMessage::ViewSyncPreCommitVote(vote) = message {
@@ -813,15 +822,18 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     self.relay += 1;
                     match last_seen_certificate {
                         ViewSyncPhase::None | ViewSyncPhase::PreCommit | ViewSyncPhase::Commit => {
-                            let vote = ViewSyncPreCommitVote::<TYPES>::create_signed_vote(
+                            let Ok(vote) = ViewSyncPreCommitVote::<TYPES>::create_signed_vote(
                                 ViewSyncPreCommitData {
                                     relay: self.relay,
                                     round: self.next_view,
                                 },
                                 self.next_view,
                                 &self.public_key,
                                 &self.private_key,
-                            );
+                            ) else {
+                                error!("Failed to sign ViewSyncPreCommitData!");
+                                return (None, self);
+                            };
                             let message =
                                 GeneralConsensusMessage::<TYPES>::ViewSyncPreCommitVote(vote);