Merge branch 'tiago/remove-max-block-time-param' (#3366)

* tiago/remove-max-block-time-param:
  Changelog for #3366
  Adjust IBC timeout values
  Clue IBC max block time estimate from Namada params
  Decrease max block time estimate to 1m
  update Hermes
  Add additional test coverage for max expected block times
  Import `namada_storage` with `testing` feat
  Avoid estimating block time of zero
  Test max block time estimates
  Insert mock block headers into test storage
  Split out `estimate_max_block_time_from_parameters`
  Remove max expected block time param
  Retrieve max block time estimate from new rpc method in masp
  Add max block duration estimate rpc method
  Read epochs per year param
  Implement date time subtraction
  Add block header Namada rpc handler

.changelog/unreleased/improvements/3366-remove-max-block-time-param.md

@@ -0,0 +1,2 @@
+- Remove the `max_expected_time_per_block` genesis parameter.
+  ([\#3366](https://github.com/anoma/namada/pull/3366))

.github/workflows/scripts/hermes.txt

@@ -1 +1 @@
-1.8.2-namada-beta11-rc2
+1.8.2-namada-beta12-rc

crates/apps_lib/src/client/rpc.rs

@@ -626,17 +626,6 @@ pub async fn query_protocol_parameters(
         epoch_duration.min_num_of_blocks
     );
 
-    let key = param_storage::get_max_expected_time_per_block_key();
-    let max_block_duration: u64 = query_storage_value(context.client(), &key)
-        .await
-        .expect("Parameter should be defined.");
-    display_line!(
-        context.io(),
-        "{:4}Max. block duration: {}",
-        "",
-        max_block_duration
-    );
-
     let key = param_storage::get_tx_allowlist_storage_key();
     let vp_allowlist: Vec<String> = query_storage_value(context.client(), &key)
         .await

crates/apps_lib/src/config/genesis.rs

@@ -18,7 +18,7 @@ use namada::core::collections::HashMap;
 use namada::core::key::*;
 use namada::core::storage;
 use namada::core::string_encoding::StringEncoded;
-use namada::core::time::{DateTimeUtc, DurationSecs};
+use namada::core::time::DateTimeUtc;
 use namada::core::token::Denomination;
 use namada::governance::parameters::GovernanceParameters;
 use namada::governance::pgf::parameters::PgfParameters;
@@ -297,8 +297,6 @@ pub struct Parameters {
     pub max_block_gas: u64,
     /// Epoch duration
     pub epoch_duration: EpochDuration,
-    /// Maximum expected time per block
-    pub max_expected_time_per_block: DurationSecs,
     /// Allowed validity predicate hashes
     pub vp_allowlist: Vec<String>,
     /// Allowed tx hashes

crates/apps_lib/src/config/genesis/chain.rs

@@ -296,7 +296,6 @@ impl Finalized {
     ) -> namada::ledger::parameters::Parameters {
         let templates::ChainParams {
             min_num_of_blocks,
-            max_expected_time_per_block,
             max_proposal_bytes,
             vp_allowlist,
             tx_allowlist,
@@ -332,16 +331,12 @@ impl Finalized {
             min_duration: namada::core::time::Duration::seconds(min_duration)
                 .into(),
         };
-        let max_expected_time_per_block =
-            namada::core::time::Duration::seconds(max_expected_time_per_block)
-                .into();
         let vp_allowlist = vp_allowlist.unwrap_or_default();
         let tx_allowlist = tx_allowlist.unwrap_or_default();
 
         namada::ledger::parameters::Parameters {
             max_tx_bytes,
             epoch_duration,
-            max_expected_time_per_block,
             vp_allowlist,
             tx_allowlist,
             implicit_vp_code_hash,

crates/apps_lib/src/config/genesis/templates.rs

@@ -265,9 +265,6 @@ pub struct ChainParams<T: TemplateValidation> {
     /// Minimum number of blocks per epoch.
     // NB: u64 only works with values up to i64::MAX with toml-rs!
     pub min_num_of_blocks: u64,
-    /// Maximum duration per block (in seconds).
-    // NB: this is i64 because datetime wants it
-    pub max_expected_time_per_block: i64,
     /// Max payload size, in bytes, for a tx batch proposal.
     ///
     /// Block proposers may never return a `PrepareProposal`
@@ -313,7 +310,6 @@ impl ChainParams<Unvalidated> {
             native_token,
             is_native_token_transferable,
             min_num_of_blocks,
-            max_expected_time_per_block,
             max_proposal_bytes,
             vp_allowlist,
             tx_allowlist,
@@ -358,7 +354,6 @@ impl ChainParams<Unvalidated> {
             native_token,
             is_native_token_transferable,
             min_num_of_blocks,
-            max_expected_time_per_block,
             max_proposal_bytes,
             vp_allowlist,
             tx_allowlist,

crates/core/src/parameters.rs

@@ -32,8 +32,6 @@ pub struct Parameters {
     pub max_tx_bytes: u32,
     /// Epoch duration (read only)
     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,
     /// Max gas for block

crates/core/src/time.rs

@@ -233,6 +233,18 @@ impl Sub<Duration> for DateTimeUtc {
     }
 }
 
+impl Sub<DateTimeUtc> for DateTimeUtc {
+    type Output = DurationSecs;
+
+    #[allow(clippy::arithmetic_side_effects)]
+    fn sub(self, rhs: DateTimeUtc) -> Self::Output {
+        (self.0 - rhs.0)
+            .to_std()
+            .map(DurationSecs::from)
+            .unwrap_or(DurationSecs(0))
+    }
+}
+
 impl BorshSerialize for DateTimeUtc {
     fn serialize<W: std::io::Write>(
         &self,

crates/ibc/src/context/common.rs

@@ -1,7 +1,5 @@
 //! IbcCommonContext implementation for IBC
 
-use core::time::Duration;
-
 use ibc::apps::nft_transfer::types::{PrefixedClassId, TokenId};
 use ibc::clients::tendermint::consensus_state::ConsensusState as TmConsensusState;
 use ibc::clients::tendermint::types::ConsensusState as TmConsensusStateType;
@@ -25,8 +23,6 @@ use ibc::primitives::Timestamp;
 use namada_core::address::Address;
 use namada_core::storage::{BlockHeight, Key};
 use namada_core::tendermint::Time as TmTime;
-use namada_core::time::DurationSecs;
-use namada_parameters::storage::get_max_expected_time_per_block_key;
 use namada_token::storage_key::balance_key;
 use namada_token::Amount;
 use prost::Message;
@@ -327,15 +323,6 @@ pub trait IbcCommonContext: IbcStorageContext {
         Ok(consensus_state.into())
     }
 
-    /// Get the max expected time per block
-    fn max_expected_time_per_block(&self) -> Result<Duration> {
-        let key = get_max_expected_time_per_block_key();
-        match self.read::<DurationSecs>(&key)? {
-            Some(duration) => Ok(duration.into()),
-            None => unreachable!("The parameter should be initialized"),
-        }
-    }
-
     /// Get the ConnectionEnd
     fn connection_end(&self, conn_id: &ConnectionId) -> Result<ConnectionEnd> {
         let key = storage::connection_key(conn_id);

crates/ibc/src/context/validation.rs

@@ -263,10 +263,28 @@ where
     }
 
     fn max_expected_time_per_block(&self) -> core::time::Duration {
-        self.inner
+        let height = self
+            .inner
             .borrow()
-            .max_expected_time_per_block()
-            .expect("Error cannot be returned")
+            .get_block_height()
+            .expect("The height should exist");
+
+        let estimate =
+            namada_parameters::estimate_max_block_time_from_blocks_and_params(
+                &*self.inner.borrow(),
+                height,
+                // NB: estimate max height with up to 5 blocks in the past,
+                // which will not result in too many reads
+                5,
+            )
+            .expect("Failed to estimate max block time");
+
+        // NB: pick a lower max blocktime estimate during tests,
+        // to avoid flakes in CI
+        #[cfg(any(test, feature = "testing"))]
+        let estimate = estimate.min(namada_core::time::DurationSecs(5));
+
+        estimate.into()
     }
 
     fn validate_message_signer(

crates/namada/src/ledger/native_vp/ibc/mod.rs

@@ -520,9 +520,7 @@ mod tests {
     use crate::ibc::{MsgNftTransfer, MsgTransfer, NftClass, NftMetadata};
     use crate::key::testing::keypair_1;
     use crate::ledger::gas::VpGasMeter;
-    use crate::ledger::parameters::storage::{
-        get_epoch_duration_storage_key, get_max_expected_time_per_block_key,
-    };
+    use crate::ledger::parameters::storage::get_epoch_duration_storage_key;
     use crate::ledger::parameters::EpochDuration;
     use crate::ledger::{ibc, pos};
     use crate::storage::{BlockHeight, TxIndex};
@@ -569,13 +567,6 @@ mod tests {
             .write_log_mut()
             .write(&epoch_duration_key, epoch_duration.serialize_to_vec())
             .expect("write failed");
-        // max_expected_time_per_block
-        let time = DurationSecs::from(Duration::new(60, 0));
-        let time_key = get_max_expected_time_per_block_key();
-        state
-            .write_log_mut()
-            .write(&time_key, namada_core::encode(&time))
-            .expect("write failed");
         // set a dummy header
         state
             .in_mem_mut()

crates/node/src/storage/mod.rs

@@ -166,7 +166,6 @@ mod tests {
                 min_num_of_blocks: 1,
                 min_duration: DurationSecs(3600),
             },
-            max_expected_time_per_block: DurationSecs(3600),
             max_proposal_bytes: Default::default(),
             max_block_gas: 100,
             vp_allowlist: vec![],

crates/parameters/Cargo.toml

@@ -24,4 +24,8 @@ namada_core = { path = "../core" }
 namada_macros = { path = "../macros" }
 namada_storage = { path = "../storage" }
 
+smooth-operator.workspace = true
 thiserror.workspace = true
+
+[dev-dependencies]
+namada_storage = { path = "../storage", features = ["testing"] }