From 4206ed3b183678468f5c3b58c6f07579fd2806f1 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sat, 22 Oct 2022 04:39:27 -0400 Subject: [PATCH] Don't import justifications multiple times Also don't broadcast blocks which were solely proposed. --- .../consensus/src/justification_import.rs | 4 +-- substrate/consensus/src/tendermint.rs | 28 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/substrate/consensus/src/justification_import.rs b/substrate/consensus/src/justification_import.rs index 7245029b..03c52ea2 100644 --- a/substrate/consensus/src/justification_import.rs +++ b/substrate/consensus/src/justification_import.rs @@ -36,9 +36,9 @@ where async fn import_justification( &mut self, hash: B::Hash, - _: ::Number, + number: ::Number, justification: Justification, ) -> Result<(), Error> { - self.import_justification_actual(hash, justification) + self.import_justification_actual(number, hash, justification) } } diff --git a/substrate/consensus/src/tendermint.rs b/substrate/consensus/src/tendermint.rs index cb88a77a..e9869611 100644 --- a/substrate/consensus/src/tendermint.rs +++ b/substrate/consensus/src/tendermint.rs @@ -1,6 +1,7 @@ use std::{ marker::PhantomData, sync::{Arc, RwLock}, + cmp::Ordering, time::Duration, }; @@ -274,9 +275,23 @@ where pub(crate) fn import_justification_actual( &mut self, + number: ::Number, hash: B::Hash, justification: Justification, ) -> Result<(), Error> { + let info = self.client.info(); + match info.best_number.cmp(&number) { + Ordering::Greater => return Ok(()), + Ordering::Equal => { + if info.best_hash == hash { + return Ok(()); + } else { + Err(Error::InvalidJustification)? + } + } + Ordering::Less => (), + } + self.verify_justification(hash, &justification)?; self .client @@ -337,7 +352,10 @@ where let (header, body) = block.clone().deconstruct(); *self.importing_block.write().unwrap() = Some(hash); self.queue.write().await.as_mut().unwrap().import_blocks( - BlockOrigin::NetworkBroadcast, + // We do not want this block, which hasn't been confirmed, to be broadcast over the net + // Substrate will generate notifications unless it's Genesis, which this isn't, InitialSync, + // which changes telemtry behavior, or File, which is... close enough + BlockOrigin::File, vec![IncomingBlock { hash, header: Some(header), @@ -361,7 +379,13 @@ where } async fn add_block(&mut self, block: B, commit: Commit) -> B { - self.import_justification_actual(block.hash(), (CONSENSUS_ID, commit.encode())).unwrap(); + self + .import_justification_actual( + *block.header().number(), + block.hash(), + (CONSENSUS_ID, commit.encode()), + ) + .unwrap(); self.get_proposal(block.header()).await } }