Skip to content

Commit

Permalink
streamline the fix
Browse files Browse the repository at this point in the history
  • Loading branch information
SuperFluffy committed Oct 24, 2024
1 parent d8de569 commit f3c6c9f
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 121 deletions.
46 changes: 11 additions & 35 deletions crates/core/component/ibc/src/component/rpc/client_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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]
Expand All @@ -35,23 +35,11 @@ impl<HI: HostInterface + Send + Sync + 'static> ClientQuery for IbcQuery<HI> {
&self,
request: tonic::Request<QueryClientStateRequest>,
) -> std::result::Result<Response<QueryClientStateResponse>, 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)
Expand Down Expand Up @@ -128,23 +116,11 @@ impl<HI: HostInterface + Send + Sync + 'static> ClientQuery for IbcQuery<HI> {
&self,
request: tonic::Request<QueryConsensusStateRequest>,
) -> std::result::Result<tonic::Response<QueryConsensusStateResponse>, 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)
Expand Down
23 changes: 6 additions & 17 deletions crates/core/component/ibc/src/component/rpc/connection_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,23 +34,12 @@ impl<HI: HostInterface + Send + Sync + 'static> ConnectionQuery for IbcQuery<HI>
request: tonic::Request<QueryConnectionRequest>,
) -> std::result::Result<tonic::Response<QueryConnectionResponse>, 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)
Expand Down
68 changes: 16 additions & 52 deletions crates/core/component/ibc/src/component/rpc/consensus_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -350,23 +350,11 @@ impl<HI: HostInterface + Send + Sync + 'static> ConsensusQuery for IbcQuery<HI>
&self,
request: tonic::Request<QueryPacketCommitmentRequest>,
) -> std::result::Result<tonic::Response<QueryPacketCommitmentResponse>, 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)
Expand Down Expand Up @@ -471,23 +459,11 @@ impl<HI: HostInterface + Send + Sync + 'static> ConsensusQuery for IbcQuery<HI>
&self,
request: tonic::Request<QueryPacketReceiptRequest>,
) -> std::result::Result<tonic::Response<QueryPacketReceiptResponse>, 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)
Expand Down Expand Up @@ -523,23 +499,11 @@ impl<HI: HostInterface + Send + Sync + 'static> ConsensusQuery for IbcQuery<HI>
request: tonic::Request<QueryPacketAcknowledgementRequest>,
) -> std::result::Result<tonic::Response<QueryPacketAcknowledgementResponse>, 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())
Expand Down
135 changes: 118 additions & 17 deletions crates/core/component/ibc/src/component/rpc/utils.rs
Original file line number Diff line number Diff line change
@@ -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<Height> {
let split: Vec<&str> = value.split('-').collect();
pub(in crate::component::rpc) fn determine_snapshot_from_height_header<T>(
storage: Storage,
request: &tonic::Request<T>,
) -> anyhow::Result<Snapshot> {
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::<u64>()
.map_err(|e| anyhow!("failed to parse revision number"))?;
fn from_str(input: &str) -> Result<Self, Self::Err> {
const FORM: &str = "input was not of the form '0' or '<number>-<height>'";

let revision_height = split[1]
.parse::<u64>()
.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::<u64>()
.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::<u64>()
.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<Height> {
let height = input
.trim()
.parse::<TheHeight>()
.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::<TheHeight>().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));
}
}

0 comments on commit f3c6c9f

Please sign in to comment.