Skip to content

Commit

Permalink
Use gRPC query ConnectionParams instead of /genesis to verify `ma…
Browse files Browse the repository at this point in the history
…x_block_time` (#4144)

* Use gRPC query to retrieve 'max_block_time' instead of querying the entire genesis data

* Add changelog entry

* Apply suggestions from code review

Co-authored-by: Romain Ruetschi <[email protected]>
Co-authored-by: Anca Zamfir <[email protected]>
Signed-off-by: Luca Joss <[email protected]>

* Override configured  if it differs from queried value

* Add debug log with queried value for 'max_expected_time_per_block'

---------

Signed-off-by: Luca Joss <[email protected]>
Co-authored-by: Romain Ruetschi <[email protected]>
Co-authored-by: Anca Zamfir <[email protected]>
  • Loading branch information
3 people authored Aug 14, 2024
1 parent 39689c7 commit f69a6d7
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 66 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Use the `ibc.core.connection.v1.ConnectionParams` gRPC query to retrieve `maxExpectedTimePerBlock`
and check it against the configured `max_block_time` instead of using the `/genesis` endpoint.
This improves both startup times and reliability for most chains.
([\#4143](https://github.com/informalsystems/hermes/issues/4143))
39 changes: 21 additions & 18 deletions crates/relayer/src/chain/cosmos.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use alloc::sync::Arc;
use core::{future::Future, str::FromStr, time::Duration};
use query::connection::query_connection_params;
use std::{cmp::Ordering, thread};

use bytes::{Buf, Bytes};
Expand All @@ -8,7 +9,7 @@ use num_bigint::BigInt;
use tokio::runtime::Runtime as TokioRuntime;
use tonic::codegen::http::Uri;
use tonic::metadata::AsciiMetadataValue;
use tracing::{debug, error, info, instrument, trace, warn};
use tracing::{debug, error, instrument, trace, warn};

use ibc_proto::cosmos::base::node::v1beta1::ConfigResponse;
use ibc_proto::cosmos::base::tendermint::v1beta1::service_client::ServiceClient;
Expand Down Expand Up @@ -113,7 +114,6 @@ use crate::util::pretty::{
use crate::HERMES_VERSION;

use self::gas::dynamic_gas_price;
use self::types::app_state::GenesisAppState;
use self::types::gas::GasConfig;
use self::version::Specs;

Expand Down Expand Up @@ -296,27 +296,30 @@ impl CosmosSdkChain {
));
}

// Query /genesis RPC endpoint to retrieve the `max_expected_time_per_block` value to use as `max_block_time`.
// If it is not found, keep the configured `max_block_time`.
match self.block_on(self.rpc_client.genesis::<GenesisAppState>()) {
Ok(genesis_reponse) => {
let old_max_block_time = self.config.max_block_time;
let new_max_block_time =
Duration::from_nanos(genesis_reponse.app_state.max_expected_time_per_block());

if old_max_block_time.as_secs() != new_max_block_time.as_secs() {
self.config.max_block_time = new_max_block_time;

info!(
"Updated `max_block_time` using /genesis endpoint. Old value: `{}s`, new value: `{}s`",
old_max_block_time.as_secs(),
self.config.max_block_time.as_secs()
// Query Connection Params with gRPC endpoint to retrieve the `max_expected_time_per_block` value and verify the
// configured `max_block_time`.
// If it is not found, the verification for the configured `max_block_time` is skipped.
match self.block_on(query_connection_params(&self.grpc_addr)) {
Ok(params) => {
debug!(
"queried `max_expected_time_per_block`: `{}ns`",
params.max_expected_time_per_block
);
let new_max_block_time = Duration::from_nanos(params.max_expected_time_per_block);

if new_max_block_time != self.config.max_block_time {
warn!(
"configured `max_block_time` value of `{}s` does not match queried value of `{}s`. \
`max_block_time` will be updated with queried value",
self.config.max_block_time.as_secs(),
new_max_block_time.as_secs(),
);
self.config.max_block_time = new_max_block_time;
}
}
Err(e) => {
warn!(
"Will use fallback value for max_block_time: `{}s`. Error: {e}",
"configured value for max_block_time: `{}s` could not be verified. Error: {e}",
self.config.max_block_time.as_secs()
);
}
Expand Down
1 change: 1 addition & 0 deletions crates/relayer/src/chain/cosmos/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::error::Error;

pub mod account;
pub mod balance;
pub mod connection;
pub mod consensus_state;
pub mod custom;
pub mod denom_trace;
Expand Down
30 changes: 30 additions & 0 deletions crates/relayer/src/chain/cosmos/query/connection.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use http::uri::Uri;

use ibc_proto::ibc::core::connection::v1::query_client::QueryClient;
use ibc_proto::ibc::core::connection::v1::Params;
use ibc_proto::ibc::core::connection::v1::QueryConnectionParamsRequest;

use crate::config::default::max_grpc_decoding_size;
use crate::error::Error;

/// Uses the GRPC client to retrieve the connection params
pub async fn query_connection_params(grpc_address: &Uri) -> Result<Params, Error> {
let mut client = QueryClient::connect(grpc_address.clone())
.await
.map_err(Error::grpc_transport)?;

client = client.max_decoding_message_size(max_grpc_decoding_size().get_bytes() as usize);

let request = tonic::Request::new(QueryConnectionParamsRequest {});

let response = client
.connection_params(request)
.await
.map(|r| r.into_inner())
.map_err(|e| Error::grpc_status(e, "query_connection_params".to_owned()))?;

// Querying connection params might not be found
let params = response.params.ok_or_else(Error::empty_connection_params)?;

Ok(params)
}
47 changes: 0 additions & 47 deletions crates/relayer/src/chain/cosmos/types/app_state.rs

This file was deleted.

1 change: 0 additions & 1 deletion crates/relayer/src/chain/cosmos/types/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
pub mod account;
pub mod app_state;
pub mod config;
pub mod events;
pub mod gas;
Expand Down
3 changes: 3 additions & 0 deletions crates/relayer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ define_error! {
EmptyUpgradedClientState
|_| { "found no upgraded client state" },

EmptyConnectionParams
|_| { "connection params not found" },

ConsensusStateTypeMismatch
{
expected: ClientType,
Expand Down

0 comments on commit f69a6d7

Please sign in to comment.