Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harden max proposal bytes configuration and deserialization #3220

Merged
merged 3 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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))
10 changes: 4 additions & 6 deletions crates/apps/src/lib/node/ledger/tendermint_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -418,10 +418,7 @@ async fn update_tendermint_config(
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;
config.mempool.max_txs_bytes = 50 * ProposalBytes::MAX.get();

// Hold up to 4k txs in the mempool
config.mempool.size = 4000;
Expand Down Expand Up @@ -475,12 +472,13 @@ 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment is now out of date

// 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,
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,
Expand Down
18 changes: 17 additions & 1 deletion crates/core/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
Loading