Merge branch 'tiago/max-proposal-bytes-validation' (#3220)

* origin/tiago/max-proposal-bytes-validation:
  Changelog for #3220
  Avoid using hardcoded values when configuring CometBFT
  Validate borsh deserialization of ProposalBytes

.changelog/unreleased/bug-fixes/3220-max-proposal-bytes-validation.md

@@ -0,0 +1,3 @@
+- Harden the implementation of `BorshDeserialize` on `ProposalBytes`.
+  Moreover, avoid using magic numbers when configuring CometBFT.
+  ([\#3220](https://github.com/anoma/namada/pull/3220))

crates/apps/src/lib/node/ledger/tendermint_node.rs

@@ -4,7 +4,7 @@ use std::process::Stdio;
 use std::str::FromStr;
 
 use borsh_ext::BorshSerializeExt;
-use namada::core::chain::ChainId;
+use namada::core::chain::{ChainId, ProposalBytes};
 use namada::core::key::*;
 use namada::core::storage::BlockHeight;
 use namada::core::time::DateTimeUtc;
@@ -417,11 +417,12 @@ async fn update_tendermint_config(
         // during some round's start
         config.mempool.max_tx_bytes = 1024 * 1024;
 
-        // Hold 50x the max amount of txs in a block
-        //
-        // 6 MiB is the default Namada max proposal size governance
-        // parameter -> 50 * 6 MiB
-        config.mempool.max_txs_bytes = 50 * 6 * 1024 * 1024;
+        // Hold 50x the max amount of txs in a block.
+        #[allow(clippy::arithmetic_side_effects)]
+        {
+            // Multiply with consts - cannot overflow
+            config.mempool.max_txs_bytes = 50 * ProposalBytes::MAX.get();
+        }
 
         // Hold up to 4k txs in the mempool
         config.mempool.size = 4000;
@@ -475,12 +476,15 @@ async fn write_tm_genesis(
             .try_into()
             .expect("Failed to convert initial genesis height");
     }
+    const EVIDENCE_AND_PROTOBUF_OVERHEAD: u64 = 10 * 1024 * 1024;
     let size = block::Size {
         // maximum size of a serialized Tendermint block.
         // on Namada, we have a hard-cap of 16 MiB (6 MiB max
         // txs in a block + 10 MiB reserved for evidence data,
-        // block headers and protobuf serialization overhead)
-        max_bytes: 16 * 1024 * 1024,
+        // block headers and protobuf serialization overhead).
+        // Addition with consts - cannot overflow.
+        #[allow(clippy::arithmetic_side_effects)]
+        max_bytes: EVIDENCE_AND_PROTOBUF_OVERHEAD + ProposalBytes::MAX.get(),
         // gas is metered app-side, so we disable it
         // at the Tendermint level
         max_gas: -1,

crates/core/src/chain.rs

@@ -2,6 +2,7 @@
 // TODO move BlockHash and BlockHeight here from the storage types
 
 use std::fmt;
+use std::io::{self, Read};
 use std::num::NonZeroU64;
 use std::str::FromStr;
 
@@ -32,13 +33,28 @@ pub const CHAIN_ID_PREFIX_SEP: char = '.';
     Hash,
     Debug,
     BorshSerialize,
-    BorshDeserialize,
     BorshDeserializer,
 )]
+#[repr(transparent)]
 pub struct ProposalBytes {
     inner: NonZeroU64,
 }
 
+impl BorshDeserialize for ProposalBytes {
+    fn deserialize_reader<R: Read>(reader: &mut R) -> std::io::Result<Self> {
+        let value: u64 = BorshDeserialize::deserialize_reader(reader)?;
+        Self::new(value).ok_or_else(|| {
+            io::Error::new(
+                io::ErrorKind::InvalidInput,
+                format!(
+                    "ProposalBytes value must be in the range 1 - {}",
+                    Self::RAW_MAX.get()
+                ),
+            )
+        })
+    }
+}
+
 impl Serialize for ProposalBytes {
     fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
     where