From dee6993ac83e85d0d2926dfe95b5ffb5843c534c Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Sat, 22 Oct 2022 06:24:39 -0400
Subject: [PATCH] Correct justication import pipeline

Removes JustificationImport as it should never be used.
---
 substrate/consensus/src/import_queue.rs       |  5 ++-
 .../consensus/src/justification_import.rs     | 44 -------------------
 substrate/consensus/src/lib.rs                |  1 -
 substrate/consensus/src/tendermint.rs         | 44 +++++--------------
 4 files changed, 13 insertions(+), 81 deletions(-)
 delete mode 100644 substrate/consensus/src/justification_import.rs

diff --git a/substrate/consensus/src/import_queue.rs b/substrate/consensus/src/import_queue.rs
index 0313b377..fe17b7e3 100644
--- a/substrate/consensus/src/import_queue.rs
+++ b/substrate/consensus/src/import_queue.rs
@@ -109,9 +109,10 @@ where
   };
 
   let boxed = Box::new(import.clone());
+  // Use None for the justification importer since justifications always come with blocks
+  // Therefore, they're never imported after the fact, mandating a importer
+  let queue = || BasicQueue::new(import.clone(), boxed.clone(), None, spawner, registry);
 
-  let queue =
-    || BasicQueue::new(import.clone(), boxed.clone(), Some(boxed.clone()), spawner, registry);
   *futures::executor::block_on(import.queue.write()) = Some(queue());
   (authority, queue())
 }
diff --git a/substrate/consensus/src/justification_import.rs b/substrate/consensus/src/justification_import.rs
deleted file mode 100644
index 03c52ea2..00000000
--- a/substrate/consensus/src/justification_import.rs
+++ /dev/null
@@ -1,44 +0,0 @@
-use async_trait::async_trait;
-
-use sp_inherents::CreateInherentDataProviders;
-use sp_runtime::{
-  traits::{Header, Block},
-  Justification,
-};
-use sp_blockchain::HeaderBackend;
-use sp_api::{TransactionFor, ProvideRuntimeApi};
-
-use sp_consensus::{Error, Environment};
-use sc_consensus::{BlockImport, JustificationImport};
-
-use sc_client_api::{Backend, Finalizer};
-
-use crate::tendermint::TendermintImport;
-
-#[async_trait]
-impl<
-    B: Block,
-    Be: Backend<B> + 'static,
-    C: Send + Sync + HeaderBackend<B> + Finalizer<B, Be> + ProvideRuntimeApi<B> + 'static,
-    I: Send + Sync + BlockImport<B, Transaction = TransactionFor<C, B>> + 'static,
-    CIDP: CreateInherentDataProviders<B, ()> + 'static,
-    E: Send + Sync + Environment<B> + 'static,
-  > JustificationImport<B> for TendermintImport<B, Be, C, I, CIDP, E>
-where
-  TransactionFor<C, B>: Send + Sync + 'static,
-{
-  type Error = Error;
-
-  async fn on_start(&mut self) -> Vec<(B::Hash, <B::Header as Header>::Number)> {
-    vec![]
-  }
-
-  async fn import_justification(
-    &mut self,
-    hash: B::Hash,
-    number: <B::Header as Header>::Number,
-    justification: Justification,
-  ) -> Result<(), Error> {
-    self.import_justification_actual(number, hash, justification)
-  }
-}
diff --git a/substrate/consensus/src/lib.rs b/substrate/consensus/src/lib.rs
index e5c31ec8..a97a91dc 100644
--- a/substrate/consensus/src/lib.rs
+++ b/substrate/consensus/src/lib.rs
@@ -15,7 +15,6 @@ mod weights;
 
 mod tendermint;
 mod block_import;
-mod justification_import;
 mod verifier;
 
 mod import_queue;
diff --git a/substrate/consensus/src/tendermint.rs b/substrate/consensus/src/tendermint.rs
index e9869611..809bb853 100644
--- a/substrate/consensus/src/tendermint.rs
+++ b/substrate/consensus/src/tendermint.rs
@@ -1,7 +1,6 @@
 use std::{
   marker::PhantomData,
   sync::{Arc, RwLock},
-  cmp::Ordering,
   time::Duration,
 };
 
@@ -162,7 +161,7 @@ where
   }
 
   // Errors if the justification isn't valid
-  fn verify_justification(
+  pub(crate) fn verify_justification(
     &self,
     hash: B::Hash,
     justification: &Justification,
@@ -192,7 +191,7 @@ where
         }
         self.verify_justification(block.header.hash(), next.unwrap())?;
 
-        block.finalized = true; // TODO: Is this setting valid?
+        block.finalized = true;
       }
     }
     Ok(())
@@ -272,32 +271,6 @@ where
       .expect("Failed to crate a new block proposal")
       .block
   }
-
-  pub(crate) fn import_justification_actual(
-    &mut self,
-    number: <B::Header as Header>::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
-      .finalize_block(BlockId::Hash(hash), Some(justification), true)
-      .map_err(|_| Error::InvalidJustification)
-  }
 }
 
 #[async_trait]
@@ -379,13 +352,16 @@ where
   }
 
   async fn add_block(&mut self, block: B, commit: Commit<TendermintSigner>) -> B {
+    let hash = block.hash();
+    let justification = (CONSENSUS_ID, commit.encode());
+    debug_assert!(self.verify_justification(hash, &justification).is_ok());
+
     self
-      .import_justification_actual(
-        *block.header().number(),
-        block.hash(),
-        (CONSENSUS_ID, commit.encode()),
-      )
+      .client
+      .finalize_block(BlockId::Hash(hash), Some(justification), true)
+      .map_err(|_| Error::InvalidJustification)
       .unwrap();
+
     self.get_proposal(block.header()).await
   }
 }