Move Commit from including the round to including the round's end_time

The round was usable to build the current clock in an accumulated 
fashion, relative to the previous round. The end time is the absolute 
metric of it, which can be used to calculate the round number (with all 
previous end times).

Substrate now builds off the best block, not genesis, using the end time 
included in the justification to start its machine in a synchronized 
state.

Knowing the end time of a round, or the round in which block was 
committed to, is necessary for nodes to sync up with Tendermint. 
Encoding it in the commit ensures it's long lasting and makes it readily 
available, without the load of an entire transaction.

substrate/consensus/src/import_queue.rs

@@ -3,9 +3,9 @@ use std::{
   sync::{Arc, RwLock},
   task::{Poll, Context},
   future::Future,
-  time::SystemTime,
 };
 
+use sp_core::Decode;
 use sp_inherents::CreateInherentDataProviders;
 use sp_runtime::traits::{Header, Block};
 use sp_api::{BlockId, TransactionFor};
@@ -18,9 +18,14 @@ use sc_client_api::Backend;
 
 use substrate_prometheus_endpoint::Registry;
 
-use tendermint_machine::{ext::BlockNumber, TendermintMachine};
+use tendermint_machine::{
+  ext::{BlockNumber, Commit},
+  TendermintMachine,
+};
 
 use crate::{
+  CONSENSUS_ID,
+  signature_scheme::TendermintSigner,
   tendermint::{TendermintClient, TendermintImport},
   Announce,
 };
@@ -98,12 +103,31 @@ where
   let authority = {
     let machine_clone = import.machine.clone();
     let mut import_clone = import.clone();
+    let best = import.client.info().best_number;
     async move {
       *machine_clone.write().unwrap() = Some(TendermintMachine::new(
         import_clone.clone(),
         // TODO
         0,
-        (BlockNumber(1), SystemTime::now()),
+        (
+          // Header::Number: TryInto<u64> doesn't implement Debug and can't be unwrapped
+          match best.try_into() {
+            Ok(best) => BlockNumber(best),
+            Err(_) => panic!("BlockNumber exceeded u64"),
+          },
+          Commit::<TendermintSigner>::decode(
+            &mut import_clone
+              .client
+              .justifications(&BlockId::Number(best))
+              .unwrap()
+              .unwrap()
+              .get(CONSENSUS_ID)
+              .unwrap()
+              .as_ref(),
+          )
+          .unwrap()
+          .end_time,
+        ),
         import_clone
           .get_proposal(&import_clone.client.header(BlockId::Number(0u8.into())).unwrap().unwrap())
           .await,

substrate/consensus/src/tendermint.rs

@@ -24,7 +24,7 @@ use sp_consensus::{Error, BlockOrigin, Proposer, Environment};
 use sc_consensus::{ForkChoiceStrategy, BlockImportParams, BlockImport, import_queue::IncomingBlock};
 
 use sc_service::ImportQueue;
-use sc_client_api::{Backend, Finalizer};
+use sc_client_api::{BlockBackend, Backend, Finalizer};
 
 use tendermint_machine::{
   ext::{BlockError, Commit, Network},
@@ -43,6 +43,7 @@ pub trait TendermintClient<B: Block, Be: Backend<B> + 'static>:
   Send
   + Sync
   + HeaderBackend<B>
+  + BlockBackend<B>
   + BlockImport<B, Transaction = TransactionFor<Self, B>>
   + Finalizer<B, Be>
   + ProvideRuntimeApi<B>
@@ -55,6 +56,7 @@ impl<
     C: Send
       + Sync
       + HeaderBackend<B>
+      + BlockBackend<B>
       + BlockImport<B, Transaction = TransactionFor<C, B>>
       + Finalizer<B, Be>
       + ProvideRuntimeApi<B>
@@ -379,8 +381,8 @@ where
       let info = self.client.info();
       assert_eq!(info.best_hash, parent);
       assert_eq!(info.finalized_hash, parent);
-      assert_eq!(info.best_number, number - 1);
+      assert_eq!(info.best_number, number - 1u8.into());
-      assert_eq!(info.finalized_number, number - 1);
+      assert_eq!(info.finalized_number, number - 1u8.into());
     }
 
     Ok(())

substrate/tendermint/src/ext.rs

@@ -64,8 +64,8 @@ pub trait SignatureScheme: Send + Sync {
 /// a valid commit.
 #[derive(Clone, PartialEq, Debug, Encode, Decode)]
 pub struct Commit<S: SignatureScheme> {
-  /// Round which created this commit.
+  /// End time of the round, used as the start time of next round.
-  pub round: Round,
+  pub end_time: u64,
   /// Validators participating in the signature.
   pub validators: Vec<S::ValidatorId>,
   /// Aggregate signature.
@@ -151,7 +151,7 @@ pub trait Network: Send + Sync {
   ) -> bool {
     if !self.signature_scheme().verify_aggregate(
       &commit.validators,
-      &commit_msg(commit.round, id.as_ref()),
+      &commit_msg(commit.end_time, id.as_ref()),
       &commit.signature,
     ) {
       return false;

substrate/tendermint/src/lib.rs

@@ -2,7 +2,7 @@ use core::fmt::Debug;
 
 use std::{
   sync::Arc,
-  time::{SystemTime, Instant, Duration},
+  time::{UNIX_EPOCH, SystemTime, Instant, Duration},
   collections::HashMap,
 };
 
@@ -24,8 +24,8 @@ use ext::*;
 mod message_log;
 use message_log::MessageLog;
 
-pub(crate) fn commit_msg(round: Round, id: &[u8]) -> Vec<u8> {
+pub(crate) fn commit_msg(end_time: u64, id: &[u8]) -> Vec<u8> {
-  [&round.0.to_le_bytes(), id].concat().to_vec()
+  [&end_time.to_le_bytes(), id].concat().to_vec()
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Encode, Decode)]
@@ -95,6 +95,7 @@ pub struct TendermintMachine<N: Network> {
   proposer: N::ValidatorId,
 
   number: BlockNumber,
+  canonical_start_time: u64,
   start_time: Instant,
   personal_proposal: N::Block,
 
@@ -126,9 +127,21 @@ pub struct TendermintHandle<N: Network> {
 }
 
 impl<N: Network + 'static> TendermintMachine<N> {
+  // Get the canonical end time for a given round, represented as seconds since the epoch
+  // While we have the Instant already in end_time, converting it to a SystemTime would be lossy,
+  // potentially enough to cause a consensus failure. Independently tracking this variable ensures
+  // that won't happen
+  fn canonical_end_time(&self, round: Round) -> u64 {
+    let mut time = self.canonical_start_time;
+    for r in 0 .. u64::from(round.0 + 1) {
+      time += (r + 1) * u64::from(N::BLOCK_TIME);
+    }
+    time
+  }
+
   fn timeout(&self, step: Step) -> Instant {
     let mut round_time = Duration::from_secs(N::BLOCK_TIME.into());
-    round_time *= self.round.0.wrapping_add(1);
+    round_time *= self.round.0 + 1;
     let step_time = round_time / 3;
 
     let offset = match step {
@@ -193,6 +206,7 @@ impl<N: Network + 'static> TendermintMachine<N> {
     sleep(round_end.saturating_duration_since(Instant::now())).await;
 
     self.number.0 += 1;
+    self.canonical_start_time = self.canonical_end_time(end_round);
     self.start_time = round_end;
     self.personal_proposal = proposal;
 
@@ -213,7 +227,7 @@ impl<N: Network + 'static> TendermintMachine<N> {
   pub fn new(
     network: N,
     proposer: N::ValidatorId,
-    start: (BlockNumber, SystemTime),
+    start: (BlockNumber, u64),
     proposal: N::Block,
   ) -> TendermintHandle<N> {
     // Convert the start time to an instant
@@ -221,7 +235,10 @@ impl<N: Network + 'static> TendermintMachine<N> {
     let start_time = {
       let instant_now = Instant::now();
       let sys_now = SystemTime::now();
-      instant_now - sys_now.duration_since(start.1).unwrap_or(Duration::ZERO)
+      instant_now -
+        sys_now
+          .duration_since(UNIX_EPOCH + Duration::from_secs(start.1))
+          .unwrap_or(Duration::ZERO)
     };
 
     let (msg_send, mut msg_recv) = mpsc::channel(100); // Backlog to accept. Currently arbitrary
@@ -239,6 +256,7 @@ impl<N: Network + 'static> TendermintMachine<N> {
           proposer,
 
           number: start.0,
+          canonical_start_time: start.1,
           start_time,
           personal_proposal: proposal,
 
@@ -318,7 +336,7 @@ impl<N: Network + 'static> TendermintMachine<N> {
                 }
 
                 let commit = Commit {
-                  round: msg.round,
+                  end_time: machine.canonical_end_time(msg.round),
                   validators,
                   signature: N::SignatureScheme::aggregate(&sigs),
                 };
@@ -349,9 +367,13 @@ impl<N: Network + 'static> TendermintMachine<N> {
     &mut self,
     msg: Message<N::ValidatorId, N::Block, <N::SignatureScheme as SignatureScheme>::Signature>,
   ) -> Result<Option<N::Block>, TendermintError<N::ValidatorId>> {
-    // Verify the signature if this is a precommit
+    // Verify the end time and signature if this is a precommit
     if let Data::Precommit(Some((id, sig))) = &msg.data {
-      if !self.signer.verify(msg.sender, &commit_msg(msg.round, id.as_ref()), sig) {
+      if !self.signer.verify(
+        msg.sender,
+        &commit_msg(self.canonical_end_time(msg.round), id.as_ref()),
+        sig,
+      ) {
         // Since we verified this validator actually sent the message, they're malicious
         Err(TendermintError::Malicious(msg.sender))?;
       }
@@ -495,7 +517,9 @@ impl<N: Network + 'static> TendermintMachine<N> {
             self.locked = Some((self.round, block.id()));
             self.broadcast(Data::Precommit(Some((
               block.id(),
-              self.signer.sign(&commit_msg(self.round, block.id().as_ref())),
+              self
+                .signer
+                .sign(&commit_msg(self.canonical_end_time(self.round), block.id().as_ref())),
             ))));
             return Ok(None);
           }