From 1607f78dbe8bd14989551dec16ed30f95b3367c7 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 24 Oct 2024 15:31:27 +0200 Subject: [PATCH] more tests --- .../ibc/src/component/rpc/client_query.rs | 6 +- .../ibc/src/component/rpc/connection_query.rs | 4 +- .../ibc/src/component/rpc/consensus_query.rs | 8 +-- .../component/ibc/src/component/rpc/utils.rs | 62 +++++++++++++++---- 4 files changed, 58 insertions(+), 22 deletions(-) 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 dc03a655d5..6865b23b76 100644 --- a/crates/core/component/ibc/src/component/rpc/client_query.rs +++ b/crates/core/component/ibc/src/component/rpc/client_query.rs @@ -26,7 +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::utils::determine_snapshot_from_metadata; use super::IbcQuery; #[async_trait] @@ -35,7 +35,7 @@ impl ClientQuery for IbcQuery { &self, request: tonic::Request, ) -> std::result::Result, Status> { - let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + let snapshot = match determine_snapshot_from_metadata(self.storage.clone(), request.metadata()) { Err(err) => return Err(tonic::Status::aborted( format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") )), @@ -116,7 +116,7 @@ impl ClientQuery for IbcQuery { &self, request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + let snapshot = match determine_snapshot_from_metadata(self.storage.clone(), request.metadata()) { Err(err) => return Err(tonic::Status::aborted( format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") )), 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 699088fd8f..2046b8be98 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::determine_snapshot_from_height_header; +use crate::component::rpc::utils::determine_snapshot_from_metadata; use crate::component::{ConnectionStateReadExt, HostInterface}; use crate::prefix::MerklePrefixExt; use crate::IBC_COMMITMENT_PREFIX; @@ -35,7 +35,7 @@ impl ConnectionQuery for IbcQuery ) -> std::result::Result, tonic::Status> { tracing::debug!("querying connection {:?}", request); - let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + let snapshot = match determine_snapshot_from_metadata(self.storage.clone(), request.metadata()) { Err(err) => return Err(tonic::Status::aborted( format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") )), 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 54bbd1a22f..f8d58dc681 100644 --- a/crates/core/component/ibc/src/component/rpc/consensus_query.rs +++ b/crates/core/component/ibc/src/component/rpc/consensus_query.rs @@ -31,7 +31,7 @@ use std::str::FromStr; use crate::component::{ChannelStateReadExt, ConnectionStateReadExt, HostInterface}; -use super::utils::determine_snapshot_from_height_header; +use super::utils::determine_snapshot_from_metadata; use super::IbcQuery; #[async_trait] @@ -350,7 +350,7 @@ impl ConsensusQuery for IbcQuery &self, request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + let snapshot = match determine_snapshot_from_metadata(self.storage.clone(), request.metadata()) { Err(err) => return Err(tonic::Status::aborted( format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") )), @@ -459,7 +459,7 @@ impl ConsensusQuery for IbcQuery &self, request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + let snapshot = match determine_snapshot_from_metadata(self.storage.clone(), request.metadata()) { Err(err) => return Err(tonic::Status::aborted( format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") )), @@ -499,7 +499,7 @@ impl ConsensusQuery for IbcQuery request: tonic::Request, ) -> std::result::Result, tonic::Status> { - let snapshot = match determine_snapshot_from_height_header(self.storage.clone(), &request) { + let snapshot = match determine_snapshot_from_metadata(self.storage.clone(), request.metadata()) { Err(err) => return Err(tonic::Status::aborted( format!("could not determine the correct snapshot to open given the `\"height\"` header of the request: {err:#}") )), diff --git a/crates/core/component/ibc/src/component/rpc/utils.rs b/crates/core/component/ibc/src/component/rpc/utils.rs index e69aa39762..642d4bcff2 100644 --- a/crates/core/component/ibc/src/component/rpc/utils.rs +++ b/crates/core/component/ibc/src/component/rpc/utils.rs @@ -8,32 +8,45 @@ use ibc_proto::ibc::core::client::v1::Height; use tracing::debug; use tracing::instrument; +type Type = tonic::metadata::MetadataMap; + +/// Determines which state snapshot to open given the height header in a [`MetadataMap`]. +/// +/// Returns the latest snapshot if the height header is 0, 0-0, or absent. #[instrument(skip_all, level = "debug")] -pub(in crate::component::rpc) fn determine_snapshot_from_height_header( +pub(in crate::component::rpc) fn determine_snapshot_from_metadata( storage: Storage, - request: &tonic::Request, + metadata: &Type, ) -> anyhow::Result { - let height = match request.metadata().get("height") { + let height = determine_height_from_metadata(metadata) + .context("failed to determine height from metadata")?; + if height.revision_height == 0 { + Ok(storage.latest_snapshot()) + } else { + storage + .snapshot(height.revision_height) + .context("failed to create state snapshot from IBC height in height header") + } +} + +#[instrument(skip_all, level = "debug")] +fn determine_height_from_metadata( + metadata: &tonic::metadata::MetadataMap, +) -> anyhow::Result { + match metadata.get("height") { None => { debug!("height header was missing; assuming a height of 0"); - TheHeight::zero().into_inner() + Ok(TheHeight::zero().into_inner()) } Some(entry) => entry .to_str() .context("height header was present but its entry was not ASCII") .and_then(parse_as_ibc_height) - .context("failed to height header as IBC height")?, - }; - if height.revision_height == 0 { - Ok(storage.latest_snapshot()) - } else { - storage - .snapshot(height.revision_height) - .context("failed to create state snapshot from IBC height in height header") + .context("failed to parse height header as IBC height"), } } -/// Utility to implement +/// Newtype wrapper around [`Height`] to implement [`FromStr`]. #[derive(Debug)] struct TheHeight(Height); @@ -90,6 +103,9 @@ fn parse_as_ibc_height(input: &str) -> anyhow::Result { #[cfg(test)] mod tests { use ibc_proto::ibc::core::client::v1::Height; + use tonic::metadata::MetadataMap; + + use crate::component::rpc::utils::determine_height_from_metadata; use super::TheHeight; @@ -121,4 +137,24 @@ mod tests { assert_ibc_height_is_parsed_correctly("1-0", height(1, 0)); assert_ibc_height_is_parsed_correctly("1-1", height(1, 1)); } + + #[track_caller] + fn assert_ibc_height_is_determined_correctly(input: Option<&str>, expected: Height) { + let mut metadata = MetadataMap::new(); + if let Some(input) = input { + metadata.insert("height", input.parse().unwrap()); + } + let actual = determine_height_from_metadata(&metadata).unwrap(); + assert_eq!(expected, actual); + } + + #[test] + fn determine_ibc_height_from_metadata() { + assert_ibc_height_is_determined_correctly(None, zero()); + assert_ibc_height_is_determined_correctly(Some("0"), zero()); + assert_ibc_height_is_determined_correctly(Some("0-0"), zero()); + assert_ibc_height_is_determined_correctly(Some("0-1"), height(0, 1)); + assert_ibc_height_is_determined_correctly(Some("1-0"), height(1, 0)); + assert_ibc_height_is_determined_correctly(Some("1-1"), height(1, 1)); + } }