diff --git a/substrate/consensus/src/import_queue.rs b/substrate/consensus/src/import_queue.rs index 64cefaa7..bf9b2ceb 100644 --- a/substrate/consensus/src/import_queue.rs +++ b/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 doesn't implement Debug and can't be unwrapped + match best.try_into() { + Ok(best) => BlockNumber(best), + Err(_) => panic!("BlockNumber exceeded u64"), + }, + Commit::::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, diff --git a/substrate/consensus/src/tendermint.rs b/substrate/consensus/src/tendermint.rs index 80bf68ee..ec6a8d6c 100644 --- a/substrate/consensus/src/tendermint.rs +++ b/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 + 'static>: Send + Sync + HeaderBackend + + BlockBackend + BlockImport> + Finalizer + ProvideRuntimeApi @@ -55,6 +56,7 @@ impl< C: Send + Sync + HeaderBackend + + BlockBackend + BlockImport> + Finalizer + ProvideRuntimeApi @@ -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.finalized_number, number - 1); + assert_eq!(info.best_number, number - 1u8.into()); + assert_eq!(info.finalized_number, number - 1u8.into()); } Ok(()) diff --git a/substrate/tendermint/src/ext.rs b/substrate/tendermint/src/ext.rs index 96536e12..6becaa7a 100644 --- a/substrate/tendermint/src/ext.rs +++ b/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 { - /// Round which created this commit. - pub round: Round, + /// End time of the round, used as the start time of next round. + pub end_time: u64, /// Validators participating in the signature. pub validators: Vec, /// 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; diff --git a/substrate/tendermint/src/lib.rs b/substrate/tendermint/src/lib.rs index f9b9de56..4a6a3884 100644 --- a/substrate/tendermint/src/lib.rs +++ b/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 { - [&round.0.to_le_bytes(), id].concat().to_vec() +pub(crate) fn commit_msg(end_time: u64, id: &[u8]) -> 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 { proposer: N::ValidatorId, number: BlockNumber, + canonical_start_time: u64, start_time: Instant, personal_proposal: N::Block, @@ -126,9 +127,21 @@ pub struct TendermintHandle { } impl TendermintMachine { + // 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 TendermintMachine { 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 TendermintMachine { pub fn new( network: N, proposer: N::ValidatorId, - start: (BlockNumber, SystemTime), + start: (BlockNumber, u64), proposal: N::Block, ) -> TendermintHandle { // Convert the start time to an instant @@ -221,7 +235,10 @@ impl TendermintMachine { 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 TendermintMachine { proposer, number: start.0, + canonical_start_time: start.1, start_time, personal_proposal: proposal, @@ -318,7 +336,7 @@ impl TendermintMachine { } 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 TendermintMachine { &mut self, msg: Message::Signature>, ) -> Result, TendermintError> { - // 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 TendermintMachine { 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); }