Provide a dedicated signature in Precommit of just the block hash

Greatly simplifies verifying when syncing.

substrate/tendermint/src/ext.rs

@@ -14,6 +14,9 @@ impl<V: Send + Sync + Clone + Copy + PartialEq + Eq + Hash + Debug + Encode + De
 {
 }
 
+pub trait Signature: Send + Sync + Clone + PartialEq + Debug + Encode + Decode {}
+impl<S: Send + Sync + Clone + PartialEq + Debug + Encode + Decode> Signature for S {}
+
 // Type aliases which are distinct according to the type system
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Encode, Decode)]
 pub struct BlockNumber(pub u32);
@@ -22,14 +25,12 @@ pub struct Round(pub u16);
 
 pub trait SignatureScheme: Send + Sync {
   type ValidatorId: ValidatorId;
-  type Signature: Send + Sync + Clone + Copy + PartialEq + Debug + Encode + Decode;
+  type Signature: Signature;
-  type AggregateSignature: Send + Sync + Clone + PartialEq + Debug + Encode + Decode;
+  type AggregateSignature: Signature;
 
   fn sign(&self, msg: &[u8]) -> Self::Signature;
   #[must_use]
   fn verify(&self, validator: Self::ValidatorId, msg: &[u8], sig: Self::Signature) -> bool;
-  // Intended to be a BLS signature, a Schnorr signature half-aggregation, or a Vec<Signature>.
-  fn aggregate(signatures: &[Self::Signature]) -> Self::AggregateSignature;
 }
 
 pub trait Weights: Send + Sync {
@@ -89,5 +90,9 @@ pub trait Network: Send + Sync {
 
   fn validate(&mut self, block: &Self::Block) -> Result<(), BlockError>;
   // Add a block and return the proposal for the next one
-  fn add_block(&mut self, block: Self::Block) -> Self::Block;
+  fn add_block(
+    &mut self,
+    block: Self::Block,
+    sigs: Vec<(Self::ValidatorId, <Self::SignatureScheme as SignatureScheme>::Signature)>,
+  ) -> Self::Block;
 }

substrate/tendermint/src/lib.rs

@@ -29,14 +29,26 @@ enum Step {
   Precommit,
 }
 
-#[derive(Clone, PartialEq, Debug, Encode, Decode)]
+#[derive(Clone, Debug, Encode, Decode)]
-enum Data<B: Block> {
+enum Data<B: Block, S: Signature> {
   Proposal(Option<Round>, B),
   Prevote(Option<B::Id>),
-  Precommit(Option<B::Id>),
+  Precommit(Option<(B::Id, S)>),
 }
 
-impl<B: Block> Data<B> {
+impl<B: Block, S: Signature> PartialEq for Data<B, S> {
+  fn eq(&self, other: &Data<B, S>) -> bool {
+    match (self, other) {
+      (Data::Proposal(r, b), Data::Proposal(r2, b2)) => (r == r2) && (b == b2),
+      (Data::Prevote(i), Data::Prevote(i2)) => i == i2,
+      (Data::Precommit(None), Data::Precommit(None)) => true,
+      (Data::Precommit(Some((i, _))), Data::Precommit(Some((i2, _)))) => i == i2,
+      _ => false,
+    }
+  }
+}
+
+impl<B: Block, S: Signature> Data<B, S> {
   fn step(&self) -> Step {
     match self {
       Data::Proposal(..) => Step::Propose,
@@ -47,18 +59,18 @@ impl<B: Block> Data<B> {
 }
 
 #[derive(Clone, PartialEq, Debug, Encode, Decode)]
-pub struct Message<V: ValidatorId, B: Block> {
+pub struct Message<V: ValidatorId, B: Block, S: Signature> {
   sender: V,
 
   number: BlockNumber,
   round: Round,
 
-  data: Data<B>,
+  data: Data<B, S>,
 }
 
 #[derive(Clone, PartialEq, Debug, Encode, Decode)]
-pub struct SignedMessage<V: ValidatorId, B: Block, S: Clone + PartialEq + Debug + Encode + Decode> {
+pub struct SignedMessage<V: ValidatorId, B: Block, S: Signature> {
-  msg: Message<V, B>,
+  msg: Message<V, B, S>,
   sig: S,
 }
 
@@ -112,7 +124,10 @@ impl<N: Network + 'static> TendermintMachine<N> {
   }
 
   #[async_recursion::async_recursion]
-  async fn broadcast(&mut self, data: Data<N::Block>) -> Option<N::Block> {
+  async fn broadcast(
+    &mut self,
+    data: Data<N::Block, <N::SignatureScheme as SignatureScheme>::Signature>,
+  ) -> Option<N::Block> {
     let step = data.step();
     let msg = Message { sender: self.proposer, number: self.number, round: self.round, data };
     let res = self.message(msg.clone()).await.unwrap();
@@ -238,7 +253,15 @@ impl<N: Network + 'static> TendermintMachine<N> {
               match machine.message(msg.msg).await {
                 Ok(None) => (),
                 Ok(Some(block)) => {
-                  let proposal = machine.network.write().await.add_block(block);
+                  let sigs = machine
+                    .log
+                    .precommitted
+                    .iter()
+                    .filter_map(|(k, (id, sig))| {
+                      Some((*k, sig.clone())).filter(|_| id == &block.id())
+                    })
+                    .collect();
+                  let proposal = machine.network.write().await.add_block(block, sigs);
                   machine.reset(proposal).await
                 }
                 Err(TendermintError::Malicious(validator)) => {
@@ -265,8 +288,9 @@ impl<N: Network + 'static> TendermintMachine<N> {
       debug_assert!(matches!(proposal, Data::Proposal(..)));
       if let Data::Proposal(_, block) = proposal {
         // Check if it has gotten a sufficient amount of precommits
-        let (participants, weight) =
+        let (participants, weight) = self
-          self.log.message_instances(round, Data::Precommit(Some(block.id())));
+          .log
+          .message_instances(round, Data::Precommit(Some((block.id(), self.signer.sign(&[])))));
 
         let threshold = self.weights.threshold();
         if weight >= threshold {
@@ -286,8 +310,14 @@ impl<N: Network + 'static> TendermintMachine<N> {
 
   async fn message(
     &mut self,
-    msg: Message<N::ValidatorId, N::Block>,
+    msg: Message<N::ValidatorId, N::Block, <N::SignatureScheme as SignatureScheme>::Signature>,
   ) -> Result<Option<N::Block>, TendermintError<N::ValidatorId>> {
+    if let Data::Precommit(Some((id, sig))) = &msg.data {
+      if !self.signer.verify(msg.sender, &id.encode(), sig.clone()) {
+        Err(TendermintError::Malicious(msg.sender))?;
+      }
+    }
+
     if msg.number != self.number {
       Err(TendermintError::Temporal)?;
     }
@@ -390,7 +420,14 @@ impl<N: Network + 'static> TendermintMachine<N> {
             self.valid = Some((self.round, block.clone()));
             if self.step == Step::Prevote {
               self.locked = self.valid.clone();
-              return Ok(self.broadcast(Data::Precommit(Some(block.id()))).await);
+              return Ok(
+                self
+                  .broadcast(Data::Precommit(Some((
+                    block.id(),
+                    self.signer.sign(&block.id().encode()),
+                  ))))
+                  .await,
+              );
             }
           }
         }

substrate/tendermint/src/message_log.rs

@@ -4,8 +4,17 @@ use crate::{ext::*, Round, Step, Data, Message, TendermintError};
 
 pub(crate) struct MessageLog<N: Network> {
   weights: Arc<N::Weights>,
-  precommitted: HashMap<N::ValidatorId, <N::Block as Block>::Id>,
+  pub(crate) precommitted: HashMap<
-  log: HashMap<Round, HashMap<N::ValidatorId, HashMap<Step, Data<N::Block>>>>,
+    N::ValidatorId,
+    (<N::Block as Block>::Id, <N::SignatureScheme as SignatureScheme>::Signature),
+  >,
+  log: HashMap<
+    Round,
+    HashMap<
+      N::ValidatorId,
+      HashMap<Step, Data<N::Block, <N::SignatureScheme as SignatureScheme>::Signature>>,
+    >,
+  >,
 }
 
 impl<N: Network> MessageLog<N> {
@@ -16,7 +25,7 @@ impl<N: Network> MessageLog<N> {
   // Returns true if it's a new message
   pub(crate) fn log(
     &mut self,
-    msg: Message<N::ValidatorId, N::Block>,
+    msg: Message<N::ValidatorId, N::Block, <N::SignatureScheme as SignatureScheme>::Signature>,
   ) -> Result<bool, TendermintError<N::ValidatorId>> {
     let round = self.log.entry(msg.round).or_insert_with(HashMap::new);
     let msgs = round.entry(msg.sender).or_insert_with(HashMap::new);
@@ -31,13 +40,13 @@ impl<N: Network> MessageLog<N> {
     }
 
     // If they already precommitted to a distinct hash, error
-    if let Data::Precommit(Some(hash)) = msg.data {
+    if let Data::Precommit(Some((hash, sig))) = &msg.data {
-      if let Some(prev) = self.precommitted.get(&msg.sender) {
+      if let Some((prev, _)) = self.precommitted.get(&msg.sender) {
-        if hash != *prev {
+        if hash != prev {
           Err(TendermintError::Malicious(msg.sender))?;
         }
       }
-      self.precommitted.insert(msg.sender, hash);
+      self.precommitted.insert(msg.sender, (*hash, sig.clone()));
     }
 
     msgs.insert(step, msg.data);
@@ -46,7 +55,11 @@ impl<N: Network> MessageLog<N> {
 
   // For a given round, return the participating weight for this step, and the weight agreeing with
   // the data.
-  pub(crate) fn message_instances(&self, round: Round, data: Data<N::Block>) -> (u64, u64) {
+  pub(crate) fn message_instances(
+    &self,
+    round: Round,
+    data: Data<N::Block, <N::SignatureScheme as SignatureScheme>::Signature>,
+  ) -> (u64, u64) {
     let mut participating = 0;
     let mut weight = 0;
     for (participant, msgs) in &self.log[&round] {
@@ -73,7 +86,11 @@ impl<N: Network> MessageLog<N> {
   }
 
   // Check if consensus has been reached on a specific piece of data
-  pub(crate) fn has_consensus(&self, round: Round, data: Data<N::Block>) -> bool {
+  pub(crate) fn has_consensus(
+    &self,
+    round: Round,
+    data: Data<N::Block, <N::SignatureScheme as SignatureScheme>::Signature>,
+  ) -> bool {
     let (_, weight) = self.message_instances(round, data);
     weight >= self.weights.threshold()
   }
@@ -83,7 +100,7 @@ impl<N: Network> MessageLog<N> {
     round: Round,
     sender: N::ValidatorId,
     step: Step,
-  ) -> Option<&Data<N::Block>> {
+  ) -> Option<&Data<N::Block, <N::SignatureScheme as SignatureScheme>::Signature>> {
     self.log.get(&round).and_then(|round| round.get(&sender).and_then(|msgs| msgs.get(&step)))
   }
 }

substrate/tendermint/tests/ext.rs

@@ -18,17 +18,13 @@ impl SignatureScheme for TestSignatureScheme {
   fn sign(&self, msg: &[u8]) -> [u8; 32] {
     let mut sig = [0; 32];
     sig[.. 2].copy_from_slice(&self.0.to_le_bytes());
-    sig[2 .. (2 + 30.min(msg.len()))].copy_from_slice(msg);
+    sig[2 .. (2 + 30.min(msg.len()))].copy_from_slice(&msg[.. 30.min(msg.len())]);
     sig
   }
 
   fn verify(&self, validator: u16, msg: &[u8], sig: [u8; 32]) -> bool {
     (sig[.. 2] == validator.to_le_bytes()) && (&sig[2 ..] == &[msg, &[0; 30]].concat()[.. 30])
   }
-
-  fn aggregate(sigs: &[[u8; 32]]) -> Vec<[u8; 32]> {
-    sigs.to_vec()
-  }
 }
 
 struct TestWeights;
@@ -95,9 +91,12 @@ impl Network for TestNetwork {
     block.valid
   }
 
-  fn add_block(&mut self, block: TestBlock) -> TestBlock {
+  fn add_block(&mut self, block: TestBlock, sigs: Vec<(u16, [u8; 32])>) -> TestBlock {
     dbg!("Adding ", &block);
     assert!(block.valid.is_ok());
+    for sig in sigs {
+      assert!(TestSignatureScheme(u16::MAX).verify(sig.0, &block.id().encode(), sig.1));
+    }
     TestBlock { id: block.id + 1, valid: Ok(()) }
   }
 }