Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4b57678
Add max block size genesis param
sug0 Nov 30, 2022
e771a3e
Add max block size param to e2e and dev genesis files
sug0 Nov 30, 2022
5144e29
Add Tendermint block size type
sug0 Nov 30, 2022
cf8444c
Tendermint block size value constructor
sug0 Nov 30, 2022
1ecff45
Add missing import
sug0 Nov 30, 2022
d557635
Add max bytes per block storage key
sug0 Nov 30, 2022
8bdbbf7
Tendermint block size type fixes
sug0 Nov 30, 2022
ca6d785
Restrict unsafe to a smaller scope
sug0 Nov 30, 2022
fadca13
Read/write block size param from/to storage
sug0 Nov 30, 2022
02f0970
Add custom serde serialization routines for tm block size
sug0 Nov 30, 2022
d1301b5
Enforce tm block size bounds during (de)serialization
sug0 Nov 30, 2022
8bc924f
WIP: Replace TendermintBytesPerBlock with ProposalSize
sug0 Dec 7, 2022
fafb161
Proposal bytes genesis param
sug0 Dec 7, 2022
0da5567
Docstr fixes
sug0 Dec 7, 2022
9daea84
Small semantics fix on gas arithmetic
sug0 Dec 7, 2022
08b6741
Fix serde value range for ProposalBytes
sug0 Dec 7, 2022
a98aae8
Write block params to Tendermint's genesis file
sug0 Dec 7, 2022
fd7cc4c
Add i64 visitor on ProposalBytes serde parser
sug0 Dec 7, 2022
2e93732
Small docstr fix
sug0 Dec 7, 2022
2c6baec
Merge eth-bridge-integration
sug0 Dec 9, 2022
f19e839
Merge branch 'eth-bridge-integration' into tiago/ethbridge/max-block-…
sug0 Dec 9, 2022
f0fc770
Add NOTE regarding TOML deserialization bug
sug0 Dec 12, 2022
0e02760
Use raw proposal bytes value in deserializer error msg
sug0 Dec 12, 2022
844f209
Fix potential open range bug whilst configuring Tendermint's max bloc…
sug0 Dec 12, 2022
878d2d3
Convert regular comments to docstrs
sug0 Dec 12, 2022
da9f3aa
Improve `max_proposal_bytes` docstr
sug0 Dec 12, 2022
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
31 changes: 25 additions & 6 deletions apps/src/lib/config/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use namada::ledger::pos::{GenesisValidator, PosParams};
use namada::types::address::{wnam, Address};
#[cfg(not(feature = "dev"))]
use namada::types::chain::ChainId;
use namada::types::chain::ProposalBytes;
use namada::types::ethereum_events::EthAddress;
use namada::types::key::dkg_session_keys::DkgPublicKey;
use namada::types::key::*;
Expand All @@ -36,6 +37,7 @@ pub mod genesis_config {
use namada::ledger::parameters::EpochDuration;
use namada::ledger::pos::{GenesisValidator, PosParams};
use namada::types::address::Address;
use namada::types::chain::ProposalBytes;
use namada::types::key::dkg_session_keys::DkgPublicKey;
use namada::types::key::*;
use namada::types::time::Rfc3339String;
Expand Down Expand Up @@ -232,17 +234,30 @@ pub mod genesis_config {

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct ParametersConfig {
// Minimum number of blocks per epoch.
/// Max payload size, in bytes, for a tx batch proposal.
///
/// Block proposers may never return a `PrepareProposal`
/// response containing `txs` with a byte length greater
/// than whatever is configured through this parameter.
///
/// Note that this parameter's value will always be strictly
/// smaller than a Tendermint block's `MaxBytes` consensus
/// parameter. Currently, we hard cap `max_proposal_bytes`
/// at 90 MiB in Namada, which leaves at least 10 MiB of
/// room for header data, evidence and protobuf
/// serialization overhead in Tendermint blocks.
pub max_proposal_bytes: ProposalBytes,
/// Minimum number of blocks per epoch.
// XXX: u64 doesn't work with toml-rs!
pub min_num_of_blocks: u64,
// Maximum duration per block (in seconds).
/// Maximum duration per block (in seconds).
// TODO: this is i64 because datetime wants it
pub max_expected_time_per_block: i64,
// Hashes of whitelisted vps array. `None` value or an empty array
// disables whitelisting.
/// Hashes of whitelisted vps array. `None` value or an empty array
/// disables whitelisting.
pub vp_whitelist: Option<Vec<String>>,
// Hashes of whitelisted txs array. `None` value or an empty array
// disables whitelisting.
/// Hashes of whitelisted txs array. `None` value or an empty array
/// disables whitelisting.
pub tx_whitelist: Option<Vec<String>>,
/// Filename of implicit accounts validity predicate WASM code
pub implicit_vp: String,
Expand Down Expand Up @@ -597,6 +612,7 @@ pub mod genesis_config {
parameters.max_expected_time_per_block,
)
.into(),
max_proposal_bytes: parameters.max_proposal_bytes,
vp_whitelist: parameters.vp_whitelist.unwrap_or_default(),
tx_whitelist: parameters.tx_whitelist.unwrap_or_default(),
implicit_vp_code_path,
Expand Down Expand Up @@ -822,6 +838,8 @@ pub struct ImplicitAccount {
BorshDeserialize,
)]
pub struct Parameters {
// Max payload size, in bytes, for a tx batch proposal.
pub max_proposal_bytes: ProposalBytes,
/// Epoch duration
pub epoch_duration: EpochDuration,
/// Maximum expected time per block
Expand Down Expand Up @@ -904,6 +922,7 @@ pub fn genesis() -> Genesis {
min_duration: namada::types::time::Duration::seconds(600).into(),
},
max_expected_time_per_block: namada::types::time::DurationSecs(30),
max_proposal_bytes: Default::default(),
vp_whitelist: vec![],
tx_whitelist: vec![],
implicit_vp_code_path: vp_implicit_path.into(),
Expand Down
2 changes: 2 additions & 0 deletions apps/src/lib/node/ledger/shell/init_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ where
// Initialize protocol parameters
let genesis::Parameters {
epoch_duration,
max_proposal_bytes,
max_expected_time_per_block,
vp_whitelist,
tx_whitelist,
Expand Down Expand Up @@ -106,6 +107,7 @@ where
}
let parameters = Parameters {
epoch_duration,
max_proposal_bytes,
max_expected_time_per_block,
vp_whitelist,
tx_whitelist,
Expand Down
12 changes: 11 additions & 1 deletion apps/src/lib/node/ledger/tendermint_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio::process::Command;

use crate::config;
use crate::facade::tendermint::Genesis;
use crate::facade::tendermint::{block, Genesis};
use crate::facade::tendermint_config::net::Address as TendermintAddress;
use crate::facade::tendermint_config::{
Error as TendermintError, TendermintConfig,
Expand Down Expand Up @@ -454,6 +454,16 @@ async fn write_tm_genesis(
genesis.genesis_time = genesis_time
.try_into()
.expect("Couldn't convert DateTimeUtc to Tendermint Time");
genesis.consensus_params.block = Some(block::Size {
// maximum size of a serialized Tendermint block
// cannot go over 100 MiB
max_bytes: (100 << 20) - 1, /* unsure if we are dealing with an open
* range, so it's better to subtract one,
* here */
// gas is metered app-side, so we disable it
// at the Tendermint level
max_gas: -1,
});
#[cfg(feature = "abcipp")]
{
genesis.consensus_params.timeout.commit =
Expand Down
57 changes: 47 additions & 10 deletions core/src/ledger/parameters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use super::storage::types::{decode, encode};
use super::storage::{types, Storage};
use crate::ledger::storage::{self as ledger_storage};
use crate::types::address::{Address, InternalAddress};
use crate::types::chain::ProposalBytes;
use crate::types::storage::Key;
use crate::types::time::DurationSecs;

Expand All @@ -32,6 +33,8 @@ pub struct Parameters {
pub epoch_duration: EpochDuration,
/// Maximum expected time per block (read only)
pub max_expected_time_per_block: DurationSecs,
/// Max payload size, in bytes, for a tx batch proposal.
pub max_proposal_bytes: ProposalBytes,
/// Whitelisted validity predicate hashes (read only)
pub vp_whitelist: Vec<String>,
/// Whitelisted tx hashes (read only)
Expand Down Expand Up @@ -101,6 +104,7 @@ impl Parameters {
let Self {
epoch_duration,
max_expected_time_per_block,
max_proposal_bytes,
vp_whitelist,
tx_whitelist,
implicit_vp,
Expand All @@ -111,6 +115,16 @@ impl Parameters {
pos_inflation_amount,
} = self;

// write max proposal bytes parameter
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add validation somewhere that the input isn't insane. Here perhaps?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or maybe it' crazy to validate genesis input. But we will need some mechanism for validation if it is changed by governance. Tomas mentioned something about guarding it with VPs. Maybe a separate pr.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we're grabbing this parameter from the toml config, whose deserialization already has some logic that checks if it is in the range [1 B, 90 MiB]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if that's enough?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I missed that. So good enough for now. For governance, I'm not so sure as it doesn't require a hard fork but just a change in storage. So we might need this VP idea. But that can be a separate PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

btw I think I misinterpreted the Tendermint specs. https://github.com/tendermint/tendermint/blob/v0.34.x/spec/abci/apps.md#blockparamsmaxbytes

I think the hard cap for a block might be 100 mb - 1 b, instead of 100 mb, since they're using an open interval

let max_proposal_bytes_key = storage::get_max_proposal_bytes_key();
let max_proposal_bytes_value = encode(&max_proposal_bytes);
storage
.write(&max_proposal_bytes_key, max_proposal_bytes_value)
.expect(
"Max proposal bytes parameter must be initialized in the \
genesis block",
);

// write epoch parameters
let epoch_key = storage::get_epoch_duration_storage_key();
let epoch_value = encode(epoch_duration);
Expand Down Expand Up @@ -382,6 +396,17 @@ where
DB: ledger_storage::DB + for<'iter> ledger_storage::DBIter<'iter>,
H: ledger_storage::StorageHasher,
{
// read max proposal bytes
let (max_proposal_bytes, gas_proposal_bytes) = {
let key = storage::get_max_proposal_bytes_key();
let (value, gas) =
storage.read(&key).map_err(ReadError::StorageError)?;
let value: ProposalBytes =
decode(value.ok_or(ReadError::ParametersMissing)?)
.map_err(ReadError::StorageTypeError)?;
(value, gas)
};

// read epoch duration
let (epoch_duration, gas_epoch) = read_epoch_duration_parameter(storage)
.expect("Couldn't read epoch duration parameters");
Expand Down Expand Up @@ -464,10 +489,31 @@ where
decode(value.ok_or(ReadError::ParametersMissing)?)
.map_err(ReadError::StorageTypeError)?;

let total_gas_cost = [
gas_epoch,
gas_tx,
gas_vp,
gas_time,
gas_implicit_vp,
gas_epy,
gas_gain_p,
gas_gain_d,
gas_staked,
gas_reward,
gas_proposal_bytes,
]
.into_iter()
.fold(0u64, |accum, gas| {
accum
.checked_add(gas)
.expect("u64 overflow occurred while doing gas arithmetic")
});

Ok((
Parameters {
epoch_duration,
max_expected_time_per_block,
max_proposal_bytes,
vp_whitelist,
tx_whitelist,
implicit_vp,
Expand All @@ -477,15 +523,6 @@ where
staked_ratio,
pos_inflation_amount,
},
gas_epoch
+ gas_tx
+ gas_vp
+ gas_time
+ gas_implicit_vp
+ gas_epy
+ gas_gain_p
+ gas_gain_d
+ gas_staked
+ gas_reward,
total_gas_cost,
))
}
28 changes: 28 additions & 0 deletions core/src/ledger/parameters/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const POS_GAIN_P_KEY: &str = "pos_gain_p";
const POS_GAIN_D_KEY: &str = "pos_gain_d";
const STAKED_RATIO_KEY: &str = "staked_ratio_key";
const POS_INFLATION_AMOUNT_KEY: &str = "pos_inflation_amount_key";
const MAX_PROPOSAL_BYTES_KEY: &str = "max_proposal_bytes";

/// Returns if the key is a parameter key.
pub fn is_parameter_key(key: &Key) -> bool {
Expand All @@ -20,10 +21,19 @@ pub fn is_parameter_key(key: &Key) -> bool {

/// Returns if the key is a protocol parameter key.
pub fn is_protocol_parameter_key(key: &Key) -> bool {
// TODO: improve this code; use some kind of prefix
// tree to efficiently match `key`
is_epoch_duration_storage_key(key)
|| is_max_expected_time_per_block_key(key)
|| is_tx_whitelist_key(key)
|| is_vp_whitelist_key(key)
|| is_implicit_vp_key(key)
|| is_epochs_per_year_key(key)
|| is_pos_gain_p_key(key)
|| is_pos_gain_d_key(key)
|| is_staked_ratio_key(key)
|| is_pos_inflation_amount_key(key)
|| is_max_proposal_bytes_key(key)
Comment on lines +24 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

oic the efficiency issue now. It would be good I guess if all protocol parameters were under some subtree e.g. /parameters/protocol/... or something, then we could easily recognize them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was playing around with trie codegen over the weekend for this specific use-case, but I still need to improve the generated code. https://gist.github.com/sug0/1bc632eb19ebdeb2d596c1617fba4138

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

btw, we do have the same internal address as a prefix in each of these keys

}

/// Returns if the key is an epoch storage key.
Expand Down Expand Up @@ -106,6 +116,14 @@ pub fn is_pos_inflation_amount_key(key: &Key) -> bool {
] if addr == &ADDRESS && pos_inflation_amount == POS_INFLATION_AMOUNT_KEY)
}

/// Returns if the key is the max proposal bytes key.
pub fn is_max_proposal_bytes_key(key: &Key) -> bool {
matches!(&key.segments[..], [
DbKeySeg::AddressSeg(addr),
DbKeySeg::StringSeg(max_proposal_bytes),
] if addr == &ADDRESS && max_proposal_bytes == MAX_PROPOSAL_BYTES_KEY)
}

/// Storage key used for epoch parameter.
pub fn get_epoch_duration_storage_key() -> Key {
Key {
Expand Down Expand Up @@ -205,3 +223,13 @@ pub fn get_pos_inflation_amount_key() -> Key {
],
}
}

/// Storage key used for the max proposal bytes.
pub fn get_max_proposal_bytes_key() -> Key {
Key {
segments: vec![
DbKeySeg::AddressSeg(ADDRESS),
DbKeySeg::StringSeg(MAX_PROPOSAL_BYTES_KEY.to_string()),
],
}
}
1 change: 1 addition & 0 deletions core/src/ledger/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,7 @@ mod tests {
..Default::default()
};
let mut parameters = Parameters {
max_proposal_bytes: Default::default(),
epoch_duration: epoch_duration.clone(),
max_expected_time_per_block: Duration::seconds(max_expected_time_per_block).into(),
vp_whitelist: vec![],
Expand Down
Loading