Implement proper checking of inherents

Cargo.lock

@@ -7328,6 +7328,7 @@ dependencies = [
  "async-trait",
  "futures",
  "log",
+ "sc-block-builder",
  "sc-client-api",
  "sc-consensus",
  "sc-executor",
@@ -8884,6 +8885,7 @@ dependencies = [
  "async-trait",
  "parity-scale-codec",
  "sp-runtime",
+ "thiserror",
  "tokio",
 ]
 

substrate/tendermint/client/Cargo.toml

@@ -37,6 +37,7 @@ sc-network = { git = "https://github.com/serai-dex/substrate" }
 sc-network-gossip = { git = "https://github.com/serai-dex/substrate" }
 sc-service = { git = "https://github.com/serai-dex/substrate" }
 sc-client-api = { git = "https://github.com/serai-dex/substrate" }
+sc-block-builder = { git = "https://github.com/serai-dex/substrate" }
 sc-consensus = { git = "https://github.com/serai-dex/substrate" }
 
 substrate-prometheus-endpoint = { git = "https://github.com/serai-dex/substrate" }

substrate/tendermint/client/src/authority/import_future.rs

@@ -7,27 +7,39 @@ use std::{
 
 use sp_runtime::traits::{Header, Block};
 
+use sp_consensus::Error;
 use sc_consensus::{BlockImportStatus, BlockImportError, Link};
 
 use sc_service::ImportQueue;
 
+use tendermint_machine::ext::BlockError;
+
 use crate::TendermintImportQueue;
 
 // Custom helpers for ImportQueue in order to obtain the result of a block's importing
-struct ValidateLink<B: Block>(Option<(B::Hash, bool)>);
+struct ValidateLink<B: Block>(Option<(B::Hash, Result<(), BlockError>)>);
 impl<B: Block> Link<B> for ValidateLink<B> {
   fn blocks_processed(
     &mut self,
     imported: usize,
     count: usize,
-    results: Vec<(
+    mut results: Vec<(
       Result<BlockImportStatus<<B::Header as Header>::Number>, BlockImportError>,
       B::Hash,
     )>,
   ) {
     assert_eq!(imported, 1);
     assert_eq!(count, 1);
-    self.0 = Some((results[0].1, results[0].0.is_ok()));
+    self.0 = Some((
+      results[0].1,
+      match results.swap_remove(0).0 {
+        Ok(_) => Ok(()),
+        Err(BlockImportError::Other(Error::Other(err))) => Err(
+          err.downcast::<BlockError>().map(|boxed| *boxed.as_ref()).unwrap_or(BlockError::Fatal),
+        ),
+        _ => Err(BlockError::Fatal),
+      },
+    ));
   }
 }
 
@@ -45,7 +57,7 @@ impl<'a, B: Block, T: Send> ImportFuture<'a, B, T> {
 }
 
 impl<'a, B: Block, T: Send> Future for ImportFuture<'a, B, T> {
-  type Output = bool;
+  type Output = Result<(), BlockError>;
 
   fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll<Self::Output> {
     let mut link = ValidateLink(None);

substrate/tendermint/client/src/authority/mod.rs

@@ -10,7 +10,6 @@ use log::warn;
 use tokio::task::yield_now;
 
 use sp_core::{Encode, Decode};
-use sp_inherents::{InherentData, InherentDataProvider, CreateInherentDataProviders};
 use sp_runtime::{
   traits::{Header, Block},
   Digest,
@@ -105,29 +104,7 @@ impl<T: TendermintValidator> TendermintAuthority<T> {
   }
 
   pub(crate) async fn get_proposal(&mut self, header: &<T::Block as Block>::Header) -> T::Block {
-    let inherent_data = match self
-      .import
-      .providers
-      .read()
-      .await
-      .as_ref()
-      .unwrap()
-      .create_inherent_data_providers(header.hash(), ())
-      .await
-    {
-      Ok(providers) => match providers.create_inherent_data() {
-        Ok(data) => Some(data),
-        Err(err) => {
-          warn!(target: "tendermint", "Failed to create inherent data: {}", err);
-          None
-        }
-      },
-      Err(err) => {
-        warn!(target: "tendermint", "Failed to create inherent data providers: {}", err);
-        None
-      }
-    }
-    .unwrap_or_else(InherentData::new);
+    let parent = *header.parent_hash();
 
     let proposer = self
       .active
@@ -137,9 +114,15 @@ impl<T: TendermintValidator> TendermintAuthority<T> {
       .init(header)
       .await
       .expect("Failed to create a proposer for the new block");
-    // TODO: Production time, size limit
+
     proposer
-      .propose(inherent_data, Digest::default(), Duration::from_secs(1), None)
+      .propose(
+        self.import.inherent_data(parent).await,
+        Digest::default(),
+        // TODO: Production time, size limit
+        Duration::from_secs(1),
+        None,
+      )
       .await
       .expect("Failed to crate a new block proposal")
       .block
@@ -316,9 +299,7 @@ impl<T: TendermintValidator> Network for TendermintAuthority<T> {
       }],
     );
 
-    if !ImportFuture::new(hash, queue_write.as_mut().unwrap()).await {
-      todo!()
-    }
+    ImportFuture::new(hash, queue_write.as_mut().unwrap()).await?;
 
     // Sanity checks that a child block can have less work than its parent
     {

substrate/tendermint/client/src/lib.rs

@@ -7,6 +7,7 @@ use sp_api::{StateBackend, StateBackendFor, TransactionFor, ApiExt, ProvideRunti
 use sp_consensus::{Error, Environment};
 
 use sc_client_api::{BlockBackend, Backend, Finalizer};
+use sc_block_builder::BlockBuilderApi;
 use sc_consensus::{BlockImport, BasicQueue};
 use sc_network::NetworkBlock;
 use sc_network_gossip::Network;
@@ -43,7 +44,9 @@ pub trait TendermintClient: Send + Sync + 'static {
     Transaction = Self::BackendTransaction,
   >;
   // Client::Api
-  type Api: ApiExt<Self::Block, StateBackend = Self::StateBackend> + TendermintApi<Self::Block>;
+  type Api: ApiExt<Self::Block, StateBackend = Self::StateBackend>
+    + BlockBuilderApi<Self::Block>
+    + TendermintApi<Self::Block>;
   type Client: Send
     + Sync
     + HeaderBackend<Self::Block>
@@ -60,7 +63,7 @@ pub trait TendermintClientMinimal: Send + Sync + 'static {
 
   type Block: Block;
   type Backend: Backend<Self::Block> + 'static;
-  type Api: ApiExt<Self::Block> + TendermintApi<Self::Block>;
+  type Api: ApiExt<Self::Block> + BlockBuilderApi<Self::Block> + TendermintApi<Self::Block>;
   type Client: Send
     + Sync
     + HeaderBackend<Self::Block>
@@ -73,7 +76,8 @@ pub trait TendermintClientMinimal: Send + Sync + 'static {
 
 impl<T: TendermintClientMinimal> TendermintClient for T
 where
-  <T::Client as ProvideRuntimeApi<T::Block>>::Api: TendermintApi<T::Block>,
+  <T::Client as ProvideRuntimeApi<T::Block>>::Api:
+    BlockBuilderApi<T::Block> + TendermintApi<T::Block>,
   TransactionFor<T::Client, T::Block>: Send + Sync + 'static,
 {
   const BLOCK_TIME_IN_SECONDS: u32 = T::BLOCK_TIME_IN_SECONDS;

substrate/tendermint/client/src/tendermint.rs

@@ -3,6 +3,8 @@ use std::{
   sync::{Arc, RwLock},
 };
 
+use log::warn;
+
 use tokio::sync::RwLock as AsyncRwLock;
 
 use sp_core::Decode;
@@ -10,12 +12,16 @@ use sp_runtime::{
   traits::{Header, Block},
   Justification,
 };
+use sp_inherents::{InherentData, InherentDataProvider, CreateInherentDataProviders};
 use sp_blockchain::HeaderBackend;
+use sp_api::{BlockId, ProvideRuntimeApi};
 
 use sp_consensus::Error;
 use sc_consensus::{ForkChoiceStrategy, BlockImportParams};
 
-use tendermint_machine::ext::{Commit, Network};
+use sc_block_builder::BlockBuilderApi;
+
+use tendermint_machine::ext::{BlockError, Commit, Network};
 
 use crate::{
   CONSENSUS_ID, TendermintValidator, validators::TendermintValidators, TendermintImportQueue,
@@ -67,9 +73,46 @@ impl<T: TendermintValidator> TendermintImport<T> {
     }
   }
 
+  pub(crate) async fn inherent_data(&self, parent: <T::Block as Block>::Hash) -> InherentData {
+    match self
+      .providers
+      .read()
+      .await
+      .as_ref()
+      .unwrap()
+      .create_inherent_data_providers(parent, ())
+      .await
+    {
+      Ok(providers) => match providers.create_inherent_data() {
+        Ok(data) => Some(data),
+        Err(err) => {
+          warn!(target: "tendermint", "Failed to create inherent data: {}", err);
+          None
+        }
+      },
+      Err(err) => {
+        warn!(target: "tendermint", "Failed to create inherent data providers: {}", err);
+        None
+      }
+    }
+    .unwrap_or_else(InherentData::new)
+  }
+
   async fn check_inherents(&self, block: T::Block) -> Result<(), Error> {
-    // TODO
-    Ok(())
+    let inherent_data = self.inherent_data(*block.header().parent_hash()).await;
+    let err = self
+      .client
+      .runtime_api()
+      .check_inherents(&BlockId::Hash(self.client.info().finalized_hash), block, inherent_data)
+      .map_err(|_| Error::Other(BlockError::Fatal.into()))?;
+
+    if err.ok() {
+      Ok(())
+    } else if err.fatal_error() {
+      Err(Error::Other(BlockError::Fatal.into()))
+    } else {
+      Err(Error::Other(BlockError::Temporal.into()))
+    }
   }
 
   // Ensure this is part of a sequential import

substrate/tendermint/machine/Cargo.toml

@@ -8,9 +8,11 @@ authors = ["Luke Parker <lukeparker5132@gmail.com>"]
 edition = "2021"
 
 [dependencies]
+async-trait = "0.1"
+thiserror = "1"
+
 parity-scale-codec = { version = "3.2", features = ["derive"] }
 
-async-trait = "0.1"
 tokio = { version = "1", features = ["macros", "sync", "time", "rt"] }
 
 sp-runtime = { git = "https://github.com/serai-dex/substrate", optional = true }

substrate/tendermint/machine/src/ext.rs

@@ -2,6 +2,7 @@ use core::{hash::Hash, fmt::Debug};
 use std::sync::Arc;
 
 use async_trait::async_trait;
+use thiserror::Error;
 
 use parity_scale_codec::{Encode, Decode};
 
@@ -97,12 +98,14 @@ pub trait Weights: Send + Sync {
 }
 
 /// Simplified error enum representing a block's validity.
-#[derive(Clone, Copy, PartialEq, Eq, Debug, Encode, Decode)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Error, Encode, Decode)]
 pub enum BlockError {
   /// Malformed block which is wholly invalid.
+  #[error("invalid block")]
   Fatal,
   /// Valid block by syntax, with semantics which may or may not be valid yet are locally
   /// considered invalid. If a block fails to validate with this, a slash will not be triggered.
+  #[error("invalid block under local view")]
   Temporal,
 }