diff --git a/crates/core/component/ibc/src/component/rpc/client_query.rs b/crates/core/component/ibc/src/component/rpc/client_query.rs index 82eaa1bdf0..dc03a655d5 100644 --- a/crates/core/component/ibc/src/component/rpc/client_query.rs +++ b/crates/core/component/ibc/src/component/rpc/client_query.rs @@ -13,7 +13,6 @@ use ibc_proto::ibc::core::client::v1::{ }; use prost::Message; -use crate::component::rpc::utils::height_from_str; use ibc_types::core::client::ClientId; use ibc_types::core::client::Height; use ibc_types::path::ClientConsensusStatePath; @@ -27,6 +26,7 @@ use crate::component::{ClientStateReadExt, HostInterface}; use crate::prefix::MerklePrefixExt; use crate::IBC_COMMITMENT_PREFIX; +use super::utils::determine_snapshot_from_height_header; use super::IbcQuery; #[async_trait] @@ -35,23 +35,11 @@ impl ClientQuery for IbcQuery { &self, request: tonic::Request, ) -> std::result::Result, Status> { - let Some(height_val) = request.metadata().get("height") else { - return Err(tonic::Status::aborted("missing height")); - }; - - let height_str: &str = height_val - .to_str() - .map_err(|e| tonic::Status::aborted(format!("invalid height: {e}")))?; - - let snapshot = if height_str == "0" { - self.storage.latest_snapshot() - } else { - let height = height_from_str(height_str) - .map_err(|e| tonic::Status::aborted(format!("couldn't get snapshot: {e}")))?; - - self.storage - .snapshot(height.revision_height as u64) - .ok_or(tonic::Status::aborted(format!("invalid height")))? + let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + Err(err) => return Err(tonic::Status::aborted( + format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") + )), + Ok(snapshot) => snapshot, }; let client_id = ClientId::from_str(&request.get_ref().client_id) @@ -128,23 +116,11 @@ impl ClientQuery for IbcQuery { &self, request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let Some(height_val) = request.metadata().get("height") else { - return Err(tonic::Status::aborted("missing height")); - }; - - let height_str: &str = height_val - .to_str() - .map_err(|e| tonic::Status::aborted(format!("invalid height: {e}")))?; - - let snapshot = if height_str == "0" { - self.storage.latest_snapshot() - } else { - let height = height_from_str(height_str) - .map_err(|e| tonic::Status::aborted(format!("couldn't get snapshot: {e}")))?; - - self.storage - .snapshot(height.revision_height as u64) - .ok_or(tonic::Status::aborted(format!("invalid height")))? + let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + Err(err) => return Err(tonic::Status::aborted( + format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") + )), + Ok(snapshot) => snapshot, }; let client_id = ClientId::from_str(&request.get_ref().client_id) diff --git a/crates/core/component/ibc/src/component/rpc/connection_query.rs b/crates/core/component/ibc/src/component/rpc/connection_query.rs index 4cf9c8009f..699088fd8f 100644 --- a/crates/core/component/ibc/src/component/rpc/connection_query.rs +++ b/crates/core/component/ibc/src/component/rpc/connection_query.rs @@ -19,7 +19,7 @@ use ibc_types::DomainType; use prost::Message; use std::str::FromStr; -use crate::component::rpc::utils::height_from_str; +use crate::component::rpc::utils::determine_snapshot_from_height_header; use crate::component::{ConnectionStateReadExt, HostInterface}; use crate::prefix::MerklePrefixExt; use crate::IBC_COMMITMENT_PREFIX; @@ -34,23 +34,12 @@ impl ConnectionQuery for IbcQuery request: tonic::Request, ) -> std::result::Result, tonic::Status> { tracing::debug!("querying connection {:?}", request); - let Some(height_val) = request.metadata().get("height") else { - return Err(tonic::Status::aborted("missing height")); - }; - - let height_str: &str = height_val - .to_str() - .map_err(|e| tonic::Status::aborted(format!("invalid height: {e}")))?; - - let snapshot = if height_str == "0" { - self.storage.latest_snapshot() - } else { - let height = height_from_str(height_str) - .map_err(|e| tonic::Status::aborted(format!("couldn't get snapshot: {e}")))?; - self.storage - .snapshot(height.revision_height as u64) - .ok_or(tonic::Status::aborted(format!("invalid height")))? + let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + Err(err) => return Err(tonic::Status::aborted( + format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") + )), + Ok(snapshot) => snapshot, }; let connection_id = &ConnectionId::from_str(&request.get_ref().connection_id) diff --git a/crates/core/component/ibc/src/component/rpc/consensus_query.rs b/crates/core/component/ibc/src/component/rpc/consensus_query.rs index 74de835849..54bbd1a22f 100644 --- a/crates/core/component/ibc/src/component/rpc/consensus_query.rs +++ b/crates/core/component/ibc/src/component/rpc/consensus_query.rs @@ -29,9 +29,9 @@ use prost::Message; use std::str::FromStr; -use crate::component::rpc::utils::height_from_str; use crate::component::{ChannelStateReadExt, ConnectionStateReadExt, HostInterface}; +use super::utils::determine_snapshot_from_height_header; use super::IbcQuery; #[async_trait] @@ -350,23 +350,11 @@ impl ConsensusQuery for IbcQuery &self, request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let Some(height_val) = request.metadata().get("height") else { - return Err(tonic::Status::aborted("missing height")); - }; - - let height_str: &str = height_val - .to_str() - .map_err(|e| tonic::Status::aborted(format!("invalid height: {e}")))?; - - let snapshot = if height_str == "0" { - self.storage.latest_snapshot() - } else { - let height = height_from_str(height_str) - .map_err(|e| tonic::Status::aborted(format!("couldn't get snapshot: {e}")))?; - - self.storage - .snapshot(height.revision_height as u64) - .ok_or(tonic::Status::aborted(format!("invalid height")))? + let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + Err(err) => return Err(tonic::Status::aborted( + format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") + )), + Ok(snapshot) => snapshot, }; let port_id = PortId::from_str(&request.get_ref().port_id) @@ -471,23 +459,11 @@ impl ConsensusQuery for IbcQuery &self, request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let Some(height_val) = request.metadata().get("height") else { - return Err(tonic::Status::aborted("missing height")); - }; - - let height_str: &str = height_val - .to_str() - .map_err(|e| tonic::Status::aborted(format!("invalid height: {e}")))?; - - let snapshot = if height_str == "0" { - self.storage.latest_snapshot() - } else { - let height = height_from_str(height_str) - .map_err(|e| tonic::Status::aborted(format!("couldn't get snapshot: {e}")))?; - - self.storage - .snapshot(height.revision_height as u64) - .ok_or(tonic::Status::aborted(format!("invalid height")))? + let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + Err(err) => return Err(tonic::Status::aborted( + format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") + )), + Ok(snapshot) => snapshot, }; let port_id = PortId::from_str(&request.get_ref().port_id) @@ -523,23 +499,11 @@ impl ConsensusQuery for IbcQuery request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let Some(height_val) = request.metadata().get("height") else { - return Err(tonic::Status::aborted("missing height")); - }; - - let height_str: &str = height_val - .to_str() - .map_err(|e| tonic::Status::aborted(format!("invalid height: {e}")))?; - - let snapshot = if height_str == "0" { - self.storage.latest_snapshot() - } else { - let height = height_from_str(height_str) - .map_err(|e| tonic::Status::aborted(format!("couldn't get snapshot: {e}")))?; - - self.storage - .snapshot(height.revision_height as u64) - .ok_or(tonic::Status::aborted(format!("invalid height")))? + let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + Err(err) => return Err(tonic::Status::aborted( + format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") + )), + Ok(snapshot) => snapshot, }; let channel_id = ChannelId::from_str(request.get_ref().channel_id.as_str()) diff --git a/crates/core/component/ibc/src/component/rpc/utils.rs b/crates/core/component/ibc/src/component/rpc/utils.rs index 108a14248a..5bb78f0fad 100644 --- a/crates/core/component/ibc/src/component/rpc/utils.rs +++ b/crates/core/component/ibc/src/component/rpc/utils.rs @@ -1,27 +1,128 @@ -use anyhow::anyhow; +use std::str::FromStr; + +use anyhow::bail; +use anyhow::Context as _; +use cnidarium::Snapshot; +use cnidarium::Storage; use ibc_proto::ibc::core::client::v1::Height; -pub(crate) fn height_from_str(value: &str) -> anyhow::Result { - let split: Vec<&str> = value.split('-').collect(); +pub(in crate::component::rpc) fn determine_snapshot_from_height_header( + storage: Storage, + request: &tonic::Request, +) -> anyhow::Result { + let height_entry = request + .metadata() + .get("height") + .context("no `height` header")? + .to_str() + .context("value of `height` header was not ASCII")?; + + 'state: { + let height = match parse_as_ibc_height(height_entry) + .context("failed to parse value as IBC height") + { + Err(err) => break 'state Err(err), + Ok(height) => height.revision_height, + }; + if height == 0 { + Ok(storage.latest_snapshot()) + } else { + storage + .snapshot(height) + .with_context(|| format!("could not open snapshot at revision height `{height}`")) + } + } + .with_context(|| { + format!("failed determine snapshot from `\"height\": \"{height_entry}` header") + }) +} + +/// Utility to implement +#[derive(Debug)] +struct TheHeight(Height); - if split.len() != 2 { - return Err(anyhow!("invalid height string")); +impl TheHeight { + fn zero() -> Self { + Self(Height { + revision_number: 0, + revision_height: 0, + }) } + fn into_inner(self) -> Height { + self.0 + } +} + +impl FromStr for TheHeight { + type Err = anyhow::Error; - let revision_number = split[0] - .parse::() - .map_err(|e| anyhow!("failed to parse revision number"))?; + fn from_str(input: &str) -> Result { + const FORM: &str = "input was not of the form '0' or '-'"; - let revision_height = split[1] - .parse::() - .map_err(|e| anyhow!("failed to parse revision height"))?; + let mut parts = input.split('-'); - if revision_number == 0 && revision_height == 0 { - return Err(anyhow!("height is zero")); + let revision_number = parts + .next() + .context(FORM)? + .parse::() + .context("failed to parse revision number as u64")?; + let revision_height = match parts.next() { + None if revision_number == 0 => return Ok(Self::zero()), + None => bail!(FORM), + Some(rev_height) => rev_height + .parse::() + .context("failed to parse revision height as u64")?, + }; + + Ok(TheHeight(Height { + revision_number, + revision_height, + })) } +} - Ok(Height { - revision_number, - revision_height, - }) +fn parse_as_ibc_height(input: &str) -> anyhow::Result { + let height = input + .trim() + .parse::() + .context("failed to parse as IBC height")? + .into_inner(); + + Ok(height) +} + +#[cfg(test)] +mod tests { + use ibc_proto::ibc::core::client::v1::Height; + + use super::TheHeight; + + fn zero() -> Height { + Height { + revision_number: 0, + revision_height: 0, + } + } + + fn height(revision_number: u64, revision_height: u64) -> Height { + Height { + revision_number, + revision_height, + } + } + + #[track_caller] + fn assert_ibc_height_is_parsed_correctly(input: &str, expected: Height) { + let actual = input.parse::().unwrap().into_inner(); + assert_eq!(expected, actual); + } + + #[test] + fn parse_ibc_height() { + assert_ibc_height_is_parsed_correctly("0", zero()); + assert_ibc_height_is_parsed_correctly("0-0", zero()); + assert_ibc_height_is_parsed_correctly("0-1", height(0, 1)); + assert_ibc_height_is_parsed_correctly("1-0", height(1, 0)); + assert_ibc_height_is_parsed_correctly("1-1", height(1, 1)); + } }