Use application latest height in query_status (informalsystems#2021)

* Test with abci_status

* Fix status also

* Remove redundant code

* Move changes in chain query_application_status()

* Cargo fmt

* Added /blockchain-based implementation for query_application_status

* Documenting impl of query_application_status.

* changelog

* Cleanup

Co-authored-by: Adi Seredinschi <[email protected]>

.changelog/unreleased/bug-fixes/relayer/1970-app-latest-height.md

@@ -0,0 +1,2 @@
+- Fixed query for application status when application lags blockchain state.
+  ([#1970](https://github.com/informalsystems/ibc-rs/issues/1970))

relayer/src/chain.rs

@@ -65,7 +65,7 @@ pub enum HealthCheck {
     Unhealthy(Box<Error>),
 }
 
-/// The result of a chain status query.
+/// The result of the application status query.
 #[derive(Clone, Debug)]
 pub struct ChainStatus {
     pub height: ICSHeight,
@@ -159,8 +159,8 @@ pub trait ChainEndpoint: Sized {
         Ok(get_compatible_versions())
     }
 
-    /// Query the latest height and timestamp the chain is at
-    fn query_status(&self) -> Result<ChainStatus, Error>;
+    /// Query the latest height and timestamp the application is at
+    fn query_application_status(&self) -> Result<ChainStatus, Error>;
 
     /// Performs a query to retrieve the state of all clients that a chain hosts.
     fn query_clients(

relayer/src/chain/cosmos.rs

@@ -21,7 +21,7 @@ use tendermint_rpc::{
 };
 use tokio::runtime::Runtime as TokioRuntime;
 use tonic::codegen::http::Uri;
-use tracing::{span, warn, Level};
+use tracing::{error, span, warn, Level};
 
 use ibc::clients::ics07_tendermint::client_state::{AllowUpdate, ClientState};
 use ibc::clients::ics07_tendermint::consensus_state::ConsensusState as TMConsensusState;
@@ -166,7 +166,7 @@ impl CosmosSdkChain {
         }
 
         // Get the latest height and convert to tendermint Height
-        let latest_height = Height::try_from(self.query_latest_height()?.revision_height)
+        let latest_height = Height::try_from(self.query_chain_latest_height()?.revision_height)
             .map_err(Error::invalid_height)?;
 
         // Check on the configured max_tx_size against the consensus parameters at latest height
@@ -356,7 +356,7 @@ impl CosmosSdkChain {
     ///
     /// Returns an error if the node is still syncing and has not caught up,
     /// ie. if `sync_info.catching_up` is `true`.
-    fn status(&self) -> Result<status::Response, Error> {
+    fn chain_status(&self) -> Result<status::Response, Error> {
         let status = self
             .block_on(self.rpc_client.status())
             .map_err(|e| Error::rpc(self.config.rpc_addr.clone(), e))?;
@@ -372,7 +372,7 @@ impl CosmosSdkChain {
     }
 
     /// Query the chain's latest height
-    pub fn query_latest_height(&self) -> Result<ICSHeight, Error> {
+    pub fn query_chain_latest_height(&self) -> Result<ICSHeight, Error> {
         crate::time!("query_latest_height");
         crate::telemetry!(query, self.id(), "query_latest_height");
 
@@ -638,16 +638,44 @@ impl ChainEndpoint for CosmosSdkChain {
             .map_err(|_| Error::ics02(ClientError::empty_prefix()))
     }
 
-    /// Query the chain status
-    fn query_status(&self) -> Result<ChainStatus, Error> {
-        crate::time!("query_status");
-        crate::telemetry!(query, self.id(), "query_status");
+    /// Query the application status
+    fn query_application_status(&self) -> Result<ChainStatus, Error> {
+        crate::time!("query_application_status");
+        crate::telemetry!(query, self.id(), "query_application_status");
 
-        self.rt.block_on(query_status(
-            self.id(),
-            &self.rpc_client,
-            &self.config.rpc_addr,
-        ))
+        // We cannot rely on `/status` endpoint to provide details about the latest block.
+        // Instead, we need to pull block height via `/abci_info` and then fetch block
+        // metadata at the given height via `/blockchain` endpoint.
+        let abci_info = self
+            .block_on(self.rpc_client.abci_info())
+            .map_err(|e| Error::rpc(self.config.rpc_addr.clone(), e))?;
+
+        // Query `/blockchain` endpoint to pull the block metadata corresponding to
+        // the latest block that the application committed.
+        // TODO: Replace this query with `/header`, once it's available.
+        //  https://github.com/informalsystems/tendermint-rs/pull/1101
+        let blocks = self
+            .block_on(
+                self.rpc_client
+                    .blockchain(abci_info.last_block_height, abci_info.last_block_height),
+            )
+            .map_err(|e| Error::rpc(self.config.rpc_addr.clone(), e))?
+            .block_metas;
+
+        return if let Some(latest_app_block) = blocks.first() {
+            let height = ICSHeight {
+                revision_number: ChainId::chain_version(latest_app_block.header.chain_id.as_str()),
+                revision_height: u64::from(abci_info.last_block_height),
+            };
+            let timestamp = latest_app_block.header.time.into();
+
+            Ok(ChainStatus { height, timestamp })
+        } else {
+            // The `/blockchain` query failed to return the header we wanted
+            Err(Error::query(
+                "/blockchain endpoint for latest app. block".to_owned(),
+            ))
+        };
     }
 
     fn query_clients(
@@ -1516,13 +1544,20 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
         return Err(Error::no_historical_entries(chain_id.clone()));
     }
 
-    let status = chain.status()?;
+    let status = chain.chain_status()?;
 
     // Check that transaction indexing is enabled
     if status.node_info.other.tx_index != TxIndexStatus::On {
         return Err(Error::tx_indexing_disabled(chain_id.clone()));
     }
 
+    // Check that the chain identifier matches the network name
+    if !status.node_info.network.as_str().eq(chain_id.as_str()) {
+        // Log the error, continue optimistically
+        error!("/status endpoint from chain id '{}' reports network identifier to be '{}': this is usually a sign of misconfiguration, check your config.toml",
+            chain_id, status.node_info.network);
+    }
+
     let version_specs = chain.block_on(fetch_version_specs(&chain.config.id, &chain.grpc_addr))?;
 
     // Checkup on the underlying SDK & IBC-go versions

relayer/src/chain/handle.rs

@@ -149,7 +149,7 @@ pub enum ChainRequest {
         reply_to: ReplyTo<Option<semver::Version>>,
     },
 
-    QueryStatus {
+    QueryApplicationStatus {
         reply_to: ReplyTo<ChainStatus>,
     },
 
@@ -384,10 +384,10 @@ pub trait ChainHandle: Clone + Send + Sync + Serialize + Debug + 'static {
     /// Return the version of the IBC protocol that this chain is running, if known.
     fn ibc_version(&self) -> Result<Option<semver::Version>, Error>;
 
-    fn query_status(&self) -> Result<ChainStatus, Error>;
+    fn query_application_status(&self) -> Result<ChainStatus, Error>;
 
     fn query_latest_height(&self) -> Result<Height, Error> {
-        Ok(self.query_status()?.height)
+        Ok(self.query_application_status()?.height)
     }
 
     fn query_clients(

relayer/src/chain/handle/base.rs

@@ -46,6 +46,8 @@ use crate::{
 
 use super::{reply_channel, ChainHandle, ChainRequest, HealthCheck, ReplyTo, Subscription};
 
+/// A basic chain handle implementation.
+/// For use in interactive CLIs, e.g., `query`, `tx raw`, etc.
 #[derive(Debug, Clone)]
 pub struct BaseChainHandle {
     /// Chain identifier
@@ -142,8 +144,8 @@ impl ChainHandle for BaseChainHandle {
         self.send(|reply_to| ChainRequest::IbcVersion { reply_to })
     }
 
-    fn query_status(&self) -> Result<ChainStatus, Error> {
-        self.send(|reply_to| ChainRequest::QueryStatus { reply_to })
+    fn query_application_status(&self) -> Result<ChainStatus, Error> {
+        self.send(|reply_to| ChainRequest::QueryApplicationStatus { reply_to })
     }
 
     fn query_clients(

relayer/src/chain/handle/cache.rs

@@ -44,6 +44,8 @@ use crate::error::Error;
 use crate::telemetry;
 use crate::{connection::ConnectionMsgType, keyring::KeyEntry};
 
+/// A chain handle with support for caching.
+/// To be used for the passive relaying mode (i.e., `start` CLI).
 #[derive(Debug, Clone)]
 pub struct CachingChainHandle<Handle> {
     inner: Handle,
@@ -127,8 +129,8 @@ impl<Handle: ChainHandle> ChainHandle for CachingChainHandle<Handle> {
         self.inner().ibc_version()
     }
 
-    fn query_status(&self) -> Result<ChainStatus, Error> {
-        self.inner().query_status()
+    fn query_application_status(&self) -> Result<ChainStatus, Error> {
+        self.inner().query_application_status()
     }
 
     fn query_latest_height(&self) -> Result<Height, Error> {

relayer/src/chain/handle/counting.rs

@@ -155,9 +155,9 @@ impl<Handle: ChainHandle> ChainHandle for CountingChainHandle<Handle> {
         self.inner().ibc_version()
     }
 
-    fn query_status(&self) -> Result<ChainStatus, Error> {
-        self.inc_metric("query_status");
-        self.inner().query_status()
+    fn query_application_status(&self) -> Result<ChainStatus, Error> {
+        self.inc_metric("query_application_status");
+        self.inner().query_application_status()
     }
 
     fn query_latest_height(&self) -> Result<Height, Error> {

relayer/src/chain/mock.rs

@@ -170,7 +170,7 @@ impl ChainEndpoint for MockChain {
         unimplemented!()
     }
 
-    fn query_status(&self) -> Result<ChainStatus, Error> {
+    fn query_application_status(&self) -> Result<ChainStatus, Error> {
         Ok(ChainStatus {
             height: self.context.host_height(),
             timestamp: self.context.host_timestamp(),

relayer/src/chain/runtime.rs

@@ -303,8 +303,8 @@ where
                             self.build_channel_proofs(port_id, channel_id, height, reply_to)?
                         },
 
-                        Ok(ChainRequest::QueryStatus { reply_to }) => {
-                            self.query_status(reply_to)?
+                        Ok(ChainRequest::QueryApplicationStatus { reply_to }) => {
+                            self.query_application_status(reply_to)?
                         }
 
                         Ok(ChainRequest::QueryClients { request, reply_to }) => {
@@ -465,8 +465,8 @@ where
         reply_to.send(result).map_err(Error::send)
     }
 
-    fn query_status(&self, reply_to: ReplyTo<ChainStatus>) -> Result<(), Error> {
-        let latest_timestamp = self.chain.query_status();
+    fn query_application_status(&self, reply_to: ReplyTo<ChainStatus>) -> Result<(), Error> {
+        let latest_timestamp = self.chain.query_application_status();
         reply_to.send(latest_timestamp).map_err(Error::send)
     }
 

relayer/src/foreign_client.rs

@@ -777,7 +777,7 @@ impl<DstChain: ChainHandle, SrcChain: ChainHandle> ForeignClient<DstChain, SrcCh
         header: &AnyHeader,
     ) -> Result<(), ForeignClientError> {
         // Get latest height and time on destination chain
-        let mut status = self.dst_chain().query_status().map_err(|e| {
+        let mut status = self.dst_chain().query_application_status().map_err(|e| {
             ForeignClientError::client_update(
                 self.dst_chain.id(),
                 "failed querying latest status of the destination chain".to_string(),
@@ -803,7 +803,7 @@ impl<DstChain: ChainHandle, SrcChain: ChainHandle> ForeignClient<DstChain, SrcCh
             let target_dst_height = status.height.increment();
             loop {
                 thread::sleep(Duration::from_millis(300));
-                status = self.dst_chain().query_status().map_err(|e| {
+                status = self.dst_chain().query_application_status().map_err(|e| {
                     ForeignClientError::client_update(
                         self.dst_chain.id(),
                         "failed querying latest status of the destination chain".to_string(),

relayer/src/link/relay_path.rs

@@ -429,7 +429,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
 
         let dst_latest_info = self
             .dst_chain()
-            .query_status()
+            .query_application_status()
             .map_err(|e| LinkError::query(self.src_chain().id(), e))?;
         let dst_latest_height = dst_latest_info.height;
         // Operational data targeting the source chain (e.g., Timeout packets)
@@ -1460,7 +1460,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
 
         let dst_status = self
             .dst_chain()
-            .query_status()
+            .query_application_status()
             .map_err(|e| LinkError::query(self.src_chain().id(), e))?;
 
         let dst_current_height = dst_status.height;

relayer/src/transfer.rs

@@ -141,14 +141,14 @@ pub fn build_and_send_transfer_messages<SrcChain: ChainHandle, DstChain: ChainHa
 
     let sender = packet_src_chain.get_signer().map_err(TransferError::key)?;
 
-    let chain_status = packet_dst_chain
-        .query_status()
+    let application_status = packet_dst_chain
+        .query_application_status()
         .map_err(TransferError::relayer)?;
 
     let timeout = TransferTimeout::new(
         opts.timeout_height_offset,
         opts.timeout_duration,
-        &chain_status,
+        &application_status,
     )?;
 
     let msg = MsgTransfer {

relayer/src/upgrade_chain.rs

@@ -71,7 +71,7 @@ pub fn build_and_send_ibc_upgrade_proposal(
     opts: &UpgradePlanOptions,
 ) -> Result<TxHash, UpgradeChainError> {
     let upgrade_height = dst_chain
-        .query_latest_height()
+        .query_chain_latest_height()
         .map_err(UpgradeChainError::query)?
         .add(opts.height_offset);
 

tools/test-framework/src/relayer/chain.rs

@@ -128,8 +128,8 @@ where
         self.value().ibc_version()
     }
 
-    fn query_status(&self) -> Result<ChainStatus, Error> {
-        self.value().query_status()
+    fn query_application_status(&self) -> Result<ChainStatus, Error> {
+        self.value().query_application_status()
     }
 
     fn query_latest_height(&self) -> Result<Height, Error> {