Move logic into TendermintImport itself

Multiple traits exist to verify/handle blocks. I'm unsure exactly when 
each will be called in the pipeline, so the easiest solution is to have 
every step run every check.

That would be extremely computationally expensive if we ran EVERY check, 
yet we rely on Substrate for execution (and according checks), which are 
limited to just the actual import function.

Since we're calling this code from many places, it makes sense for it to 
be consolidated under TendermintImport.

substrate/consensus/src/import_queue.rs

@@ -1,4 +1,3 @@
-use std::{marker::PhantomData, sync::Arc, collections::HashMap};
 // The Tendermint machine will call add_block for any block which is committed to, regardless of
 // validity. To determine validity, it expects a validate function, which Substrate doesn't
 // directly offer, and an add function. In order to comply with Serai's modified view of inherent
@@ -12,45 +11,97 @@ use std::{marker::PhantomData, sync::Arc, collections::HashMap};
 //
 // When Tendermint completes, the block is finalized, setting it as the tip regardless of work.
 
+use std::{
+  marker::PhantomData,
+  sync::{Arc, RwLock},
+  collections::HashMap,
+};
+
+use async_trait::async_trait;
+
+use tokio::sync::RwLock as AsyncRwLock;
+
 use sp_core::Decode;
+use sp_application_crypto::sr25519::Signature;
 use sp_inherents::CreateInherentDataProviders;
-use sp_runtime::traits::{Header, Block};
+use sp_runtime::{
+  traits::{Header, Block},
+  Justification,
+};
 use sp_blockchain::HeaderBackend;
-use sp_api::{ProvideRuntimeApi, TransactionFor};
+use sp_api::{BlockId, TransactionFor, ProvideRuntimeApi};
 
 use sp_consensus::{Error, CacheKeyId};
-#[rustfmt::skip]
+#[rustfmt::skip] // rustfmt doesn't know how to handle this line
 use sc_consensus::{
   ForkChoiceStrategy,
   BlockCheckParams,
   BlockImportParams,
   ImportResult,
   BlockImport,
+  JustificationImport,
+  BasicQueue,
 };
 
-use tendermint_machine::ext::*;
+use sc_client_api::{Backend, Finalizer};
+
+use substrate_prometheus_endpoint::Registry;
+
+use tendermint_machine::{
+  ext::{BlockError, Commit, Network},
+  SignedMessage,
+};
+
+use crate::{signature_scheme::TendermintSigner, weights::TendermintWeights};
 
-use crate::signature_scheme::TendermintSigner;
 const CONSENSUS_ID: [u8; 4] = *b"tend";
 
-struct TendermintBlockImport<
+struct TendermintImport<
   B: Block,
-  C: Send + Sync + HeaderBackend<B> + ProvideRuntimeApi<B> + 'static,
-  I: Send + Sync + BlockImport<B, Transaction = TransactionFor<C, B>>,
-  CIDP: CreateInherentDataProviders<B, ()>,
+  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,
 > {
   _block: PhantomData<B>,
+  _backend: PhantomData<Be>,
+
+  importing_block: Arc<RwLock<Option<B::Hash>>>,
+
   client: Arc<C>,
-  inner: I,
+  inner: Arc<AsyncRwLock<I>>,
   providers: Arc<CIDP>,
 }
 
 impl<
     B: Block,
-    C: Send + Sync + HeaderBackend<B> + ProvideRuntimeApi<B>,
-    I: Send + Sync + BlockImport<B, Transaction = TransactionFor<C, B>>,
-    CIDP: CreateInherentDataProviders<B, ()>,
-  > TendermintBlockImport<B, C, I, CIDP>
+    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,
+  > Clone for TendermintImport<B, Be, C, I, CIDP>
+{
+  fn clone(&self) -> Self {
+    TendermintImport {
+      _block: PhantomData,
+      _backend: PhantomData,
+
+      importing_block: self.importing_block.clone(),
+
+      client: self.client.clone(),
+      inner: self.inner.clone(),
+      providers: self.providers.clone(),
+    }
+  }
+}
+
+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,
+  > TendermintImport<B, Be, C, I, CIDP>
 {
   async fn check_inherents(
     &self,
@@ -59,59 +110,92 @@ impl<
   ) -> Result<(), Error> {
     todo!()
   }
-}
 
-#[async_trait::async_trait]
-impl<
-    B: Block,
-    C: Send + Sync + HeaderBackend<B> + ProvideRuntimeApi<B>,
-    I: Send + Sync + BlockImport<B, Transaction = TransactionFor<C, B>>,
-    CIDP: CreateInherentDataProviders<B, ()>,
-  > BlockImport<B> for TendermintBlockImport<B, C, I, CIDP>
-where
-  I::Error: Into<Error>,
-{
-  type Error = Error;
-  type Transaction = TransactionFor<C, B>;
-
-  async fn check_block(
-    &mut self,
-    mut block: BlockCheckParams<B>,
-  ) -> Result<ImportResult, Self::Error> {
+  // Ensure this is part of a sequential import
+  fn verify_order(
+    &self,
+    parent: B::Hash,
+    number: <B::Header as Header>::Number,
+  ) -> Result<(), Error> {
     let info = self.client.info();
-    if (info.best_hash != block.parent_hash) || ((info.best_number + 1u16.into()) != block.number) {
+    if (info.best_hash != parent) || ((info.best_number + 1u16.into()) != number) {
       Err(Error::Other("non-sequential import".into()))?;
     }
-
-    block.allow_missing_state = false;
-    block.allow_missing_parent = false;
-
-    self.inner.check_block(block).await.map_err(Into::into)
+    Ok(())
   }
 
-  async fn import_block(
-    &mut self,
-    mut block: BlockImportParams<B, Self::Transaction>,
-    new_cache: HashMap<CacheKeyId, Vec<u8>>,
-  ) -> Result<ImportResult, Self::Error> {
-    if let Some(body) = block.body.clone() {
-      if let Some(justifications) = block.justifications {
+  // Do not allow blocks from the traditional network to be broadcast
+  // Only allow blocks from Tendermint
+  // Tendermint's propose message could be rewritten as a seal OR Tendermint could produce blocks
+  // which this checks the proposer slot for, and then tells the Tendermint machine
+  // While those would be more seamless with Substrate, there's no actual benefit to doing so
+  fn verify_origin(&self, hash: B::Hash) -> Result<(), Error> {
+    if let Some(tm_hash) = *self.importing_block.read().unwrap() {
+      if hash == tm_hash {
+        return Ok(());
+      }
+    }
+    Err(Error::Other("block created outside of tendermint".into()))
+  }
+
+  // Errors if the justification isn't valid
+  fn verify_justification(
+    &self,
+    hash: B::Hash,
+    justification: &Justification,
+  ) -> Result<(), Error> {
+    if justification.0 != CONSENSUS_ID {
+      Err(Error::InvalidJustification)?;
+    }
+
+    let commit: Commit<TendermintSigner> =
+      Commit::decode(&mut justification.1.as_ref()).map_err(|_| Error::InvalidJustification)?;
+    if !self.verify_commit(hash, &commit) {
+      Err(Error::InvalidJustification)?;
+    }
+    Ok(())
+  }
+
+  // Verifies the justifications aren't malformed, not that the block is justified
+  // Errors if justifications is neither empty nor a sinlge Tendermint justification
+  // If the block does have a justification, finalized will be set to true
+  fn verify_justifications<T>(&self, block: &mut BlockImportParams<B, T>) -> Result<(), Error> {
+    if !block.finalized {
+      if let Some(justifications) = &block.justifications {
         let mut iter = justifications.iter();
         let next = iter.next();
         if next.is_none() || iter.next().is_some() {
           Err(Error::InvalidJustification)?;
         }
-        let justification = next.unwrap();
+        self.verify_justification(block.header.hash(), next.unwrap())?;
 
-        let commit: Commit<TendermintSigner> =
-          Commit::decode(&mut justification.1.as_ref()).map_err(|_| Error::InvalidJustification)?;
-        if justification.0 != CONSENSUS_ID {
-          Err(Error::InvalidJustification)?;
-        }
+        block.finalized = true; // TODO: Is this setting valid?
+      }
+    }
+    Ok(())
+  }
 
-        // verify_commit
-        todo!()
-      } else {
+  async fn check<T>(&self, block: &mut BlockImportParams<B, T>) -> Result<(), Error> {
+    if block.finalized {
+      if block.fork_choice.is_none() {
+        // Since we alw1ays set the fork choice, this means something else marked the block as
+        // finalized, which shouldn't be possible. Ensuring nothing else is setting blocks as
+        // finalized ensures our security
+        panic!("block was finalized despite not setting the fork choice");
+      }
+      return Ok(());
+    }
+
+    // Set the block as a worse choice
+    block.fork_choice = Some(ForkChoiceStrategy::Custom(false));
+
+    self.verify_order(*block.header.parent_hash(), *block.header.number())?;
+    self.verify_justifications(block)?;
+
+    // If the block wasn't finalized, verify the origin and validity of its inherents
+    if !block.finalized {
+      self.verify_origin(block.header.hash())?;
+      if let Some(body) = block.body.clone() {
         self
           .check_inherents(
             B::new(block.header.clone(), body),
@@ -121,6 +205,9 @@ where
       }
     }
 
+    // Additionally check these fields are empty
+    // They *should* be unused, so requiring their emptiness prevents malleability and ensures
+    // nothing slips through
     if !block.post_digests.is_empty() {
       Err(Error::Other("post-digests included".into()))?;
     }
@@ -128,7 +215,6 @@ where
       Err(Error::Other("auxiliary included".into()))?;
     }
 
-    block.fork_choice = Some(ForkChoiceStrategy::Custom(false));
-    self.inner.import_block(block, new_cache).await.map_err(Into::into)
+    Ok(())
   }
 }