From 32ad6de00cf645ee5fb25520555256bdd17de547 Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Fri, 11 Nov 2022 06:38:06 -0500
Subject: [PATCH] Properly define and pass around the block size

---
 substrate/node/src/service.rs                    | 11 +++++++++--
 substrate/runtime/src/lib.rs                     |  5 ++++-
 substrate/tendermint/client/src/authority/mod.rs |  3 +--
 substrate/tendermint/client/src/lib.rs           | 13 ++++++++++---
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/substrate/node/src/service.rs b/substrate/node/src/service.rs
index 43646965..abb3fcda 100644
--- a/substrate/node/src/service.rs
+++ b/substrate/node/src/service.rs
@@ -18,7 +18,7 @@ pub(crate) use sc_tendermint::{
   TendermintClientMinimal, TendermintValidator, TendermintImport, TendermintAuthority,
   TendermintSelectChain, import_queue,
 };
-use serai_runtime::{self, TARGET_BLOCK_TIME, opaque::Block, RuntimeApi};
+use serai_runtime::{self, BLOCK_SIZE, TARGET_BLOCK_TIME, opaque::Block, RuntimeApi};
 
 type FullBackend = sc_service::TFullBackend<Block>;
 pub type FullClient = TFullClient<Block, RuntimeApi, NativeElseWasmExecutor<ExecutorDispatch>>;
@@ -63,6 +63,10 @@ impl CreateInherentDataProviders<Block, ()> for Cidp {
 
 pub struct TendermintValidatorFirm;
 impl TendermintClientMinimal for TendermintValidatorFirm {
+  // TODO: This is passed directly to propose, which warns not to use the hard limit as finalize
+  // may grow the block. We don't use storage proofs and use the Executive finalize_block. Is that
+  // guaranteed not to grow the block?
+  const PROPOSED_BLOCK_SIZE_LIMIT: usize = { BLOCK_SIZE as usize };
   // 3 seconds
   const BLOCK_PROCESSING_TIME_IN_SECONDS: u32 = { (TARGET_BLOCK_TIME / 2 / 1000) as u32 };
   // 1 second
@@ -178,7 +182,10 @@ pub async fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceE
     config.chain_spec.fork_id(),
   );
   if is_authority {
-    config.network.extra_sets.push(sc_tendermint::set_config(tendermint_protocol.clone()));
+    config
+      .network
+      .extra_sets
+      .push(sc_tendermint::set_config(tendermint_protocol.clone(), BLOCK_SIZE.into()));
   }
 
   let (network, system_rpc_tx, tx_handler_controller, network_starter) =
diff --git a/substrate/runtime/src/lib.rs b/substrate/runtime/src/lib.rs
index 1be68ff2..9747950b 100644
--- a/substrate/runtime/src/lib.rs
+++ b/substrate/runtime/src/lib.rs
@@ -81,6 +81,9 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
   state_version: 1,
 };
 
+// 1 MB
+pub const BLOCK_SIZE: u32 = 1024 * 1024;
+// 6 seconds
 pub const TARGET_BLOCK_TIME: u64 = 6000;
 
 /// Measured in blocks.
@@ -111,7 +114,7 @@ parameter_types! {
 
   // 1 MB block size limit
   pub BlockLength: frame_system::limits::BlockLength =
-    frame_system::limits::BlockLength::max_with_normal_ratio(1024 * 1024, NORMAL_DISPATCH_RATIO);
+    frame_system::limits::BlockLength::max_with_normal_ratio(BLOCK_SIZE, NORMAL_DISPATCH_RATIO);
   pub BlockWeights: frame_system::limits::BlockWeights =
     frame_system::limits::BlockWeights::with_sensible_defaults(
       (2u64 * WEIGHT_PER_SECOND).set_proof_size(u64::MAX),
diff --git a/substrate/tendermint/client/src/authority/mod.rs b/substrate/tendermint/client/src/authority/mod.rs
index 957bb655..a191e7e8 100644
--- a/substrate/tendermint/client/src/authority/mod.rs
+++ b/substrate/tendermint/client/src/authority/mod.rs
@@ -132,8 +132,7 @@ impl<T: TendermintValidator> TendermintAuthority<T> {
         Digest::default(),
         // Assumes a block cannot take longer to download than it'll take to process
         Duration::from_secs((T::BLOCK_PROCESSING_TIME_IN_SECONDS / 2).into()),
-        // TODO: Size limit
-        None,
+        Some(T::PROPOSED_BLOCK_SIZE_LIMIT),
       )
       .await
       .expect("Failed to crate a new block proposal")
diff --git a/substrate/tendermint/client/src/lib.rs b/substrate/tendermint/client/src/lib.rs
index 880b27a4..4ebf6e47 100644
--- a/substrate/tendermint/client/src/lib.rs
+++ b/substrate/tendermint/client/src/lib.rs
@@ -44,15 +44,20 @@ pub fn protocol_name<Hash: AsRef<[u8]>>(genesis: Hash, fork: Option<&str>) -> Pr
   name.into()
 }
 
-pub fn set_config(protocol: ProtocolName) -> NonDefaultSetConfig {
-  // TODO: 1 MiB Block Size + 1 KiB
-  let mut cfg = NonDefaultSetConfig::new(protocol, (1024 * 1024) + 1024);
+pub fn set_config(protocol: ProtocolName, block_size: u64) -> NonDefaultSetConfig {
+  // The extra 512 bytes is for the additional data part of Tendermint
+  // Even with BLS, that should just be 161 bytes in the worst case, for a perfect messaging scheme
+  // While 256 bytes would suffice there, it's unknown if any LibP2P overhead exists nor if
+  // anything here will be perfect. Considering this is miniscule compared to the block size, it's
+  // better safe than sorry.
+  let mut cfg = NonDefaultSetConfig::new(protocol, block_size + 512);
   cfg.allow_non_reserved(25, 25);
   cfg
 }
 
 /// Trait consolidating all generics required by sc_tendermint for processing.
 pub trait TendermintClient: Send + Sync + 'static {
+  const PROPOSED_BLOCK_SIZE_LIMIT: usize;
   const BLOCK_PROCESSING_TIME_IN_SECONDS: u32;
   const LATENCY_TIME_IN_SECONDS: u32;
 
@@ -82,6 +87,7 @@ pub trait TendermintClient: Send + Sync + 'static {
 
 /// Trait implementable on firm types to automatically provide a full TendermintClient impl.
 pub trait TendermintClientMinimal: Send + Sync + 'static {
+  const PROPOSED_BLOCK_SIZE_LIMIT: usize;
   const BLOCK_PROCESSING_TIME_IN_SECONDS: u32;
   const LATENCY_TIME_IN_SECONDS: u32;
 
@@ -104,6 +110,7 @@ where
     BlockBuilderApi<T::Block> + TendermintApi<T::Block>,
   TransactionFor<T::Client, T::Block>: Send + Sync + 'static,
 {
+  const PROPOSED_BLOCK_SIZE_LIMIT: usize = T::PROPOSED_BLOCK_SIZE_LIMIT;
   const BLOCK_PROCESSING_TIME_IN_SECONDS: u32 = T::BLOCK_PROCESSING_TIME_IN_SECONDS;
   const LATENCY_TIME_IN_SECONDS: u32 = T::LATENCY_TIME_IN_SECONDS;